Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Use clean contexts for IO Thread callbacks#21327

Closed
benaadams wants to merge 1 commit intodotnet:masterfrom
benaadams:Clean-contexts-for-IO-Thread-callbacks
Closed

Use clean contexts for IO Thread callbacks#21327
benaadams wants to merge 1 commit intodotnet:masterfrom
benaadams:Clean-contexts-for-IO-Thread-callbacks

Conversation

@benaadams
Copy link
Member

@benaadams benaadams commented Dec 2, 2018

For PerformIOCompletionCallback skips the use of _IOCompletionCallback when ExecutionContext flow is suppressed or Default.

However, if any changes were made by the callback to the ExecutionContext; they would not be undone, nor fire change notifications with ThreadContextChanged == true; additionally those changes would then flow into the next flow suppressed callback or Default context callback.

This change starts the PerformIOCompletionCallback thread on the Default (null) context; then after each loop iteration it also undoes any changes made in the callback to the context; including firing ThreadContextChanged == true notifications; so they no longer can flow into the next iteration or activation of PerformIOCompletionCallback.

Additionally it includes the optimization from #21320 of only looking up the Thread.CurrentThread once per loop; and using the fast-path ExecutionContext.Run that the ThreadPool uses (renamed in this from RunForThreadPoolUnsafe to RunForThreadLoopUnsafe).

/cc @stephentoub @jkotas @kouvel

@benaadams
Copy link
Member Author

benaadams commented Dec 2, 2018

Starting on the default context here and for the ThreadPool with

// Start on clean ExecutionContext and SynchronizationContext
currentThread.ExecutionContext = null;
currentThread.SynchronizationContext = null;

Is probably overly pessimistic; but not sure if the threads get used elsewhere as there are a number of callback types into the runtime from unmanaged code.

@benaadams
Copy link
Member Author

benaadams commented Dec 2, 2018

Note: this wouldn't resolve what I was trying to address in #21320 because SocketEventArgs captures its own contexts then the run here is flow supressed so it still goes via the full ExecutionContext.Run which then looks up Thread.CurrentThread each time.

However, already issue for the SocketEventArgs capture dotnet/corefx#32582

@stephentoub stephentoub requested a review from kouvel March 15, 2019 16:33
@benaadams benaadams force-pushed the Clean-contexts-for-IO-Thread-callbacks branch from 24db90d to 7fdb446 Compare April 7, 2019 15:24
@benaadams
Copy link
Member Author

Broke something in the rebase; though I'm not strongly attached to this change

@benaadams benaadams closed this Apr 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant