Reset certain thread-local state after thread pool work item execution

[GSPP]

It can sometimes happen by accident that a thread pool work item sets some thread-local state such as:

• SynchronizationContext.Current
• HostContext (used for HttpContext.Current)
• Thread priority
• Thread name
• Thread culture
• Thread principal
• Others?

Failing to reset that state causes non-deterministic "leaks" that are very hard to diagnose. This is especially relevant in ASP.NET and in unit testing scenarios. The thread pool is a global process resource. Generally, code should play nice with such resources but bugs should not be catastrophic.

The .NET thread pool should reset all of these items (where practical) after the execution of each work item. I see no valid scenario why someone might want to set these values and have them persist because there is no way to make that deterministic.

Regarding the above items I could not find documentation that specifies which one of them are reset or are not reset. This behavior should be documented and contractually guaranteed.

Ideally, setting Thread properties should fail outright but that might not be possible for compatibility reasons.

The behavior changes proposed here should work for the legacy thread pool API as well as for the TPL.

I have not added the execution context and logical call context to the list because I lack the technical understanding to recommend anything about them. Maybe they should be reset as well.

I have not added ThreadStatic and other forms of thread-static variables because those are often useful for caching scenarios. Resetting them would be a bad idea.

[HaloFour]

Honestly I'd think it's worth it to reset all TLS and treat each work item as if it's executing on a new thread.

[danmoseley]

You're probably right, but this would be a significant breaking change, right?

[GSPP]

The change should not cause much breakage. If someone had relied on this his code only spuriously had the behavior that we are trying to remove here. His code saw mixed behavior. Sometimes the state leaks, sometimes it's clean.

Therefore, his app must be able to tolerate thread local state disappearing at any time. If it now always disappears (instead of sometimes) that should be fine.

The only case this would cause breakage is if there was some very deterministic thread scheduling for some reason that made the thread local state deterministic in practice. Such a thing is hard to imagine. Certainly next to impossible in a web app where requests arrive randomly.

I consider resetting the state as proposed quite a high value change.

[GSPP]

Further, should we maybe add an API that allows anyone to reset thread state? That would be useful for custom TaskSchedulers. Maybe something like TaskCompletionSource and Task where the "source" acts like a capability to configure the Task. Here's a sketch for how a custom TaskScheduler could use that API:

ThreadSource ts = new ThreadSource(ThreadProc);
Thread thread = ts.Thread; //worker thread
thread.Start();

void ThreadProc() {
 while (true) {
  var workItem = Dequeue();
  Run(workItem);
  ts.ResetState(); //Reset using privileged object.
 }
}

The ThreadSource would be kept private by the TaskScheduler. It could be used to perform administrative actions that are not supposed to be exposed to work item code.

Alternative: A Thread.ResetState() method that anyone can call.

[kouvel]

The work tasks perform is not expected to rely on thread state. They could potentially use thread locals for caching purposes since a thread pool is expected to reuse threads, and that would be valid. But aside from that, they are not expected to modify thread state that could otherwise negatively affect other tasks. If a task is doing such a thing, to me that would seem to be the problem.

If a task needed to modify thread state for its own benefit, I would expect that it revert any changes it made before it completes, if such a change could negatively affect other tasks or the system as a whole. If this happens accidentally, I would consider that a bug.

@GSPP:

I consider resetting the state as proposed quite a high value change.

Could you please elaborate on that? I would like to see a valid scenario for this.

[GSPP]

@kouvel I did not mean to suggest that not resetting the state in the application code is a good thing. I agree it should be considered a bug. But what if such a bug happens? It's catastrophic:

Failing to reset that state causes non-deterministic "leaks" that are very hard to diagnose.

For that reason .NET apps become more stable if the thread pool always resets threads.

Flipping this around: Assume that pool threads were resetting their state today. Would we support a change for them to not reset their state? Of course not, that's absurd because this is clearly worse.

I would like to see a valid scenario for this.

I don't understand what you mean. I do not propose to not reset pool state. In my post I warn of doing that and I advocate systematically resetting. Maybe we misunderstood one another.

[kouvel]

But what if such a bug happens? It's catastrophic: Failing to reset that state causes non-deterministic "leaks" that are very hard to diagnose.

There are ways to diagnose these types of problems, they're not impossible to diagnose. I don't think hiding the problem is a good solution.

Therefore, his app must be able to tolerate thread local state disappearing at any time. If it now always disappears (instead of sometimes) that should be fine.

State changes could be made to be fairly deterministic, for instance if the change is common to all threads. Resetting them could cause perf issues in the app, which could be considered a break. Although the thread-specific state would disappear, the user may have global state that indicates that the current thread should already have the state changes, which could lead to functional problems. I think this has to be considered a breaking change.

Flipping this around: Assume that pool threads were resetting their state today. Would we support a change for them to not reset their state? Of course not, that's absurd because this is clearly worse.

Certainly could consider such a change in the interest of performance. Work items are often extremely short and any addition to that path could cause a noticeable degradation in throughput depending on how much resetting is being done. But I would initially choose to not reset state mainly because it places an arbitrary restriction without a good reason for doing so, while there are some good reasons for not doing so.

Alternative: A Thread.ResetState() method that anyone can call.

Something like this may be good for diagnosis. Maybe a flags enum parameter to indicate which states to reset. It would be nice to see what sort of problems are happening and how prevalent these problems are, for considering this API.

[alexperovich]

The Thread.ResetState() method with a flags enum parameter makes sense. Can we get a formal api proposal?

[akazemis]

I need this fix. Is anyone working on it? or is there any other ticket for this fix?

[alexperovich]

Nobody is working on this right now.

[dlennert]

We currently use ThreadLocal values in thread pool threads to store values that, for performance, we prefer not to manage statically with locking. E.g., the Random class is not thread safe, but a unique instance stored in ThreadLocal effectively is. Sure we code things to tolerate it disappearing from ThreadLocal, but that defeats the performance goal.

(And please don't suggest that I use a random number generator classes which is thread safe; that's not the point of my example. Random is simply a well-known, non-thread-safe class I can use as an example.)

[stephentoub]

No proposal has been made for APIs related to this in a couple of years, and we're not going to change behaviors here by default. I'm going to close this.