2

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!)

Improve this question
5
  • 1
    As an aside, you may want to use a SynchronizedStream. – Brian Oct 26 '15 at 19:57
  • You also need to implement locking (mutual exclusion) to protect a sequence of related operations. For streams, related operations are usually "seek - read/write (possibly repeated)". – rwong Oct 26 '15 at 21:28
  • 2
    The term for this is called "multiplexing" a stream. Each reader takes turn setting its position and reading its data; while preventing overlapping requests. It does depend on the stream not having any "magic properties", i.e. anything that makes it not like a typical stream. – rwong Oct 26 '15 at 21:34
  • How will you deal with writes? Or is your subclass read-only? Especially concurrent writes, and the easier case, writes concurrent with reads. – Ⴖuі Oct 27 '15 at 6:03
  • 1
    Thanks for the comments! I didn't know of SynchronizedStream, so that was good to hear of. To clarify - I didn't want to clutter the question but the real problem requires slightly more than multiplexing (its something more along the lines of a cache, so the threads don't know they are sharing a stream - and in fact they aren't - they are sharing a block of memory written to by another thread which does own the stream). Writes are therefore not an issue, but the Position state is, since it can't be multiplexed. – sebf Oct 29 '15 at 19:46
6

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+).

Improve this answer
3
  • +1 This should be the accepted answer. Seems to me that this would get you both the "ease of use" (less boilerplate) and explicitness I like. – Marjan Venema Oct 27 '15 at 9:01
  • All these answers have been great! Unfortunately I am not in .NET 4.5 for this project, so cannot use ThreadLocal<>, however I am glad to see that thread-specific functionality is considered an acceptable pattern, enough to be explicitly supported by .NET, and this is the approach I will take in the future. On reflection I agree with everyone here that explicitness for future maintainers trumps cleanliness. Where specific types like ThreadLocal<> are available I will prefer that, where they aren't, I will, and have, implemented a non-threadsafe 'view' class into a thread-safe object. – sebf Oct 29 '15 at 19:46
  • Note that ThreadLocal<T> support began on .NET 4.0. ThreadStatic support began with .Net 1.1 . If you're not on .NET 4.0+, I see no reason not to just use ThreadStatic. – Brian Oct 30 '15 at 1:08
7

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.
Improve this answer
2
  • 1
    Microsoft created an internal, thread-safe implementation of Stream. However, it's implemented in the standard, boring way; Every method on SyncStream merely calls the internal stream, with all such calls (excluding calls to pure functions) being wrapped in a lock. This is what you get if you call Stream.Synchronized. – Brian Oct 26 '15 at 20:22
  • 3
    The contract of Stream is well-defined, and given that, what the OP is proposing is properly termed "multiplexing" multiple readers onto a single instance of Stream. As long as the multiplexing does not introduce threading errors, it is considered a safe practice. But I agree that in general, i.e. when not talking about Streams but something else, one cannot make the same assurance, so it should be avoided. – rwong Oct 26 '15 at 21:31
2

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.

Improve this answer
2
  • The OP is using C#, which offers ThreadStatic and ThreadLocal<T>. It is up to the coder whether they are global (static) or instance-specific. The OP would want an instance-specific variable, since there could be multiple instances of the stream, each used by one or more threads. – Brian Oct 26 '15 at 20:15
  • Thanks @Brian! Didn't about that feature. Love to learn. And seems to me that this would get you both the "ease of use" (less boilerplate) and explicitness I like. – Marjan Venema Oct 27 '15 at 9:01
0

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

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.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.