Fix panic in defer after cache eviction. (#1048)

We have rather rare but annoying panic that hapens when workflow
contains certain api calls within a defer function. This only happens
when workflow is being blocked for some time and gets evicted from
the cache. At this point the dispatcher is being closed and all its
coroutines are exited with runtime.Goexit(). This in turn calls deffered
function. If user calls ExecuteActivity (among others) it receives
context that is no longer valid, since it is used not from within
ExecuteUntilAllBlocked loop. A safety check in getState is executed
which causes rather cryptic panic message: "illegal access from
outside of workflow context".

This panic is not harmful. Once unblocked workflow will be recreated
in the cache and replayed. When finished it will call its defer again
this time with a valid context. Nevertheless it is annoying an causes
confusion for the users.

The fix is rather simple. Within getState function, we check for closed
dispatcher. If it is the case we terminate current goroutine again with
runtime.GoExit() which prevent further execution of the defer.

Author: vytautas-karpavicius

internal/internal_coroutines_test.go

@@ -626,6 +626,11 @@ func TestDispatchClose(t *testing.T) {
 		for i := 0; i < 10; i++ {
 			ii := i
 			GoNamed(ctx, fmt.Sprintf("c-%v", i), func(ctx Context) {
+				defer func(){
+					// Trigger no-longer-valid context access within deferred function.
+					// At this point deferred function should not continue and will be exited.
+					getState(ctx)
+				}()
 				c.Receive(ctx, nil) // blocked forever
 				history = append(history, fmt.Sprintf("child-%v", ii))
 			})
@@ -652,6 +657,10 @@ func TestDispatchClose(t *testing.T) {
 		"root",
 	}
 	require.EqualValues(t, expected, history)
+	for _, c := range d.coroutines {
+		// Ensure that coroutines did not panic during dispatcher closing procedure
+		require.Nil(t, c.panicError)
+	}
 }
 
 func TestPanic(t *testing.T) {

internal/internal_workflow.go

@@ -564,6 +564,12 @@ func getState(ctx Context) *coroutineState {
 		panic("getState: not workflow context")
 	}
 	state := s.(*coroutineState)
+	// When workflow gets evicted from cache is closes the dispatcher and exits all its coroutines.
+	// However if workflow function have a defer, it will be executed. Many workflow API calls will end up here.
+	// The following check prevents coroutine executing further. It would panic otherwise as context is no longer valid.
+	if state.dispatcher.closed {
+		runtime.Goexit()
+	}
 	if !state.dispatcher.executing {
 		panic(panicIllegalAccessCoroutinueState)
 	}