Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a subtle one! There was a race between two thread managers: the
System.TimeManager.Manager
and theNetwork.HTTP2.H2.Manager
(which wraps the former). The race condition could arise as follows:Suppose an exception is raised in one of the background threads (e.g., the frame sender).
Then
stopAfter
writes aStop
instruction theH2.Manager
queue; the exception gets then rethrown.If the server is run in the scope of
withManager
(as it is done inNetwork.Run.TCP.Timeout.runTCPServerWithSocket
for example), the exception is caught again, causingstopManager
to be called.Now one of two things can happen first (for each managed thread):
a.
stopManager
might call the timeout action, which will throwTimeoutThread
to the thread (because we registered the threads withregisterKillThread
)b. The
H2.Manager
will process theStop
instruction, and throwKilledByHTTP2ThreadManager
to the thread.The choice is non-deterministic, depending on thread scheduling by the RTS.
After this, the other exception is then thrown to same thread (which might at this point be busy handling the first exception).
So not only is the type of exception that is thrown non-deterministic, we also have the possibility that the thread will be interrupted handling that first exception by the second exception.
This commit fixes this by putting the
H2.Manager
in charge (it is aware of both managers, after all). WhenstopAfter
writesStop
to theH2.Manager
queue, it waits until thatStop
instruction has been processed; this will (after this commit) also ensure that the threads have been removed from the timeout manager. This guarantees that the exception that is delivered (and the only exception that is delivered) is theKilledByHTTP2ThreadManager
fromhttp2
.