Is it a good idea to make method behavior depend on the calling thread?

Question

I want to subclass a 3rd party class, in order to make it thread-safe.

I have a good idea of how to implement this, but there is a problem: the superclass has a property, which affects the behaviour of one of its methods. If one thread sets the property, it will interfere with the other threads when they call the method.

I can see two ways to do this:

1. Create a thread-safe 'stateless' object which then has multiple 'views' into it. The property is in the view and each thread has its own view instance.
2. Detect which thread makes the call in the property's get accessor and the method, and store the state for that thread internally.

(1) is self-explanatory, but it involves more boilerplate code. (2) does something non-trivial behind the scenes, but if it works, it is completely transparent.

Which is best, for maintainability and readability? The more complex code but whose behaviour is up-front, or the code which is easier to use when it works, but if it breaks it will do so in a location and way which is not obvious?

Is there any reason an object should not be dependent on what thread interacts with it?

(EDIT: Removing reference to the 3rd party class, since the requirements of the implementation are not as simple as it sounds and it was generating more confusion than needed!)

Answer 1

The idea of having a thread-specific variable is not unreasonable, though I am unsure if it is appropriate for your use case. The idea of a thread-safe Stream strikes me as a bit broken; I would rather have a thread-safe StreamFactory.

The best way to implement a thread-specific state variable is to use either ThreadStatic or ThreadLocal<T>. This makes your code short, simple, and trivially maintainable. This variable will be a member of your Stream.

See ThreadStatic v.s. ThreadLocal: is generic better than attribute? for discussion on which to use (short version: use ThreadLocal<T> if you're on .Net 4+).

Answer 2

This does not answer your question, but:

1. It is not a good idea to subclass a 3rd party class, in order to make it thread-safe. Author most likely did not think of thread safety when designing the class, so it will be very hard to add it by subclassing, if even possible. Thinking about polymorphism and thread safety at the same time is just too much. What if the implementation of the class will change?
2. It is not a good idea to make a thread-safe stream. Stream can not be read by two readers at the same time, so why bother to access it from multiple threads? Accidental complexity.

Answer 3

Interesting. In Delphi you can declare variables as threadvar which ensures that every thread has its own copy. However, these are global vars and thus have their drawbacks. So a colleague came up with "thread-stance" data. Data that is both thread and instance specific. Very similar to your option 2. It worked and works beautifully.

Given the choice today, I would go for option 1.

While option 2 may make things completely transparent for your fellow developers, I don't think that is necessarily a good thing. It comes at the cost of obscuring the "thread-dependent" nature of that class. Something a developer should always be aware of.

Option 1 makes everything much more explicit, and me, I like explicit.

Answer 4

Stating the obvious, creating a synchronized stream is just-about completely pointless¹.

It isn't that you cannot ensure the steams internals aren't mangled by multi-threading, that's actually really easy.
The problem comes from the fact that just almost all code relies on implicit exclusive access to the stream to ensure that conceptually atomic pieces of output aren't arbitrarily interrupted and interrupting each other, which will make the output flat-out useless.

If multiple threads can access the same thread, they rely on you, the programmer, sensibly synchronizing access in logical units anyway.

Regarding the exceptions, that's for the few times when either you ensure that output will be moved to the stream in logical units (everyone being on-board with it, or you using a forwarding stream which will not ever auto-flush), or things are so broken you can only write your error-message, hope for the best, and abort anyway.