Do not call callbacks for deleted contexts. (#270)

Signed-off-by: Takeshi Yoneda <[email protected]>
tetratelabs · Mar 23, 2022 · dff2a62 · dff2a62
1 parent 0dc1df1
commit dff2a62
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 26 deletions.
diff --git a/proxywasm/internal/abi_callback_l7.go b/proxywasm/internal/abi_callback_l7.go
@@ -91,8 +91,18 @@ func proxyOnHttpCallResponse(pluginContextID, calloutID uint32, numHeaders, body
 		panic("invalid callout id")
 	}
 
-	ProxySetEffectiveContext(cb.callerContextID)
-	currentState.setActiveContextID(cb.callerContextID)
+	ctxID := cb.callerContextID
+	currentState.setActiveContextID(ctxID)
 	delete(root.httpCallbacks, calloutID)
-	cb.callback(numHeaders, bodySize, numTrailers)
+
+	// Check if the context is already deleted.
+	// For example, if the connection expired before the call response arrival,
+	// proxy_on_http_call_response is called AFTER ProxyOnDelete is called for the context id.
+	// In that case, if the callback continues response or make local reply, then the subsequent
+	// callbacks (for example OnHttpResponseHeaders) would follow and result in calling callback
+	// for already-deleted context id. See https://github.com/tetratelabs/proxy-wasm-go-sdk/issues/261 for detail.
+	if _, ok := currentState.contextIDToRootID[ctxID]; ok {
+		ProxySetEffectiveContext(ctxID)
+		cb.callback(numHeaders, bodySize, numTrailers)
+	}
 }
diff --git a/proxywasm/internal/abi_callback_l7_test.go b/proxywasm/internal/abi_callback_l7_test.go
@@ -96,33 +96,47 @@ func Test_proxyOnHttpCallResponse(t *testing.T) {
 
 	var (
 		pluginContextID uint32 = 1
+		callerContextID uint32 = 100
 		callOutID       uint32 = 10
 	)
 
 	currentStateMux.Lock()
 	defer currentStateMux.Unlock()
 
-	ctx := &l7Context{}
-	currentState = &state{
-		pluginContexts: map[uint32]*pluginContextState{pluginContextID: {
-			httpCallbacks: map[uint32]*httpCallbackAttribute{callOutID: {callback: ctx.OnHttpCallResponse}},
-		}},
-	}
-
-	proxyOnHttpCallResponse(pluginContextID, callOutID, 0, 0, 0)
-	_, ok := currentState.pluginContexts[pluginContextID].httpCallbacks[callOutID]
-	require.False(t, ok)
-	require.True(t, ctx.onHttpCallResponse)
-
-	ctx = &l7Context{}
-	currentState = &state{
-		pluginContexts: map[uint32]*pluginContextState{pluginContextID: {
-			httpCallbacks: map[uint32]*httpCallbackAttribute{callOutID: {callback: ctx.OnHttpCallResponse}},
-		}},
-	}
-
-	proxyOnHttpCallResponse(pluginContextID, callOutID, 0, 0, 0)
-	_, ok = currentState.pluginContexts[pluginContextID].httpCallbacks[callOutID]
-	require.False(t, ok)
-	require.True(t, ctx.onHttpCallResponse)
+	t.Run("normal", func(t *testing.T) {
+		ctx := &l7Context{}
+		currentState = &state{
+			pluginContexts: map[uint32]*pluginContextState{pluginContextID: {
+				httpCallbacks: map[uint32]*httpCallbackAttribute{callOutID: {callback: ctx.OnHttpCallResponse, callerContextID: callerContextID}},
+			}},
+			contextIDToRootID: map[uint32]uint32{callerContextID: pluginContextID},
+		}
+
+		proxyOnHttpCallResponse(pluginContextID, callOutID, 0, 0, 0)
+		_, ok := currentState.pluginContexts[pluginContextID].httpCallbacks[callOutID]
+		require.False(t, ok)
+		require.True(t, ctx.onHttpCallResponse)
+	})
+
+	t.Run("delete before callback", func(t *testing.T) {
+		ctx := &l7Context{}
+		currentState = &state{
+			pluginContexts: map[uint32]*pluginContextState{pluginContextID: {
+				httpCallbacks: map[uint32]*httpCallbackAttribute{callOutID: {callback: ctx.OnHttpCallResponse, callerContextID: callerContextID}},
+			}},
+			httpContexts:      map[uint32]types.HttpContext{callerContextID: nil},
+			contextIDToRootID: map[uint32]uint32{callerContextID: pluginContextID},
+		}
+
+		proxyOnDelete(callerContextID)
+
+		proxyOnHttpCallResponse(pluginContextID, callOutID, 0, 0, 0)
+		_, ok := currentState.pluginContexts[pluginContextID].httpCallbacks[callOutID]
+		require.False(t, ok)
+
+		// If the caller context is deleted before callback is called, then
+		// the callback shouldn't be called.
+		require.False(t, ctx.onHttpCallResponse)
+	})
+
 }