9

I'm writing a .NET library which exposes certain public APIs. Currently, I have not enforced thread safety in my library for following reasons apparent to me:

  • locks (Monitor.Enter and Monitor.Exit) are only limited to the AppDomain, so they do not provide synchronization in the inter-process situations.
  • Mutex can be used bigger scope more than the AppDomain, but then I need to struggle with OS-specific limitations such as naming rules for named mutexes.
  • Both lock and Mutex can be useless and cause performance costs if the consumer never wanted to use synchronization.
  • Consumers might need different synchronization mechanism such as Semaphore.
  • MSDN's Managed threading best practices for class libraries states that avoid synchronization and not make instance data thread safe by default.

From these what I understood was, when it comes to class libraries, it is the consumers' responsibility to enforce the thread safety in their own way when using that library and the developer should not worry about it.

Is my understanding correct? especially when writing any class library? Also, what would your approach when documenting the library? Should we explicitly state the absence of thread-safety or let the consumers assume that it is not thread safe?

6
  • 7
    The MSDN article is more subtle than you summarized here. It recommends to avoid the need for synchronization, and does expect you to make concurrent use of global data safe – just not to bother with making individual instances thread-safe as a user of your APIs could easily guard access to the object via a mutex of their own. But it all depends on what your classes are actually doing. Some things cannot be made thread-safe from within your library, other classes would be useless if not specifically designed to be concurrency-safe.
    – amon
    Commented May 7 at 11:02
  • 8
    As an example of pointless threadsafety: consider a dictionary- or repository-like type in which methods like dict.Contains(key), dict.Set(key, value) and dict.Get(key) are individually thread-safe. But this only gives a false sense of safety for a user who combines multiple of these operations, e.g. dict.Contains(key) ? dict.Set(key, 1 + dict.Get(key)) : dict.Set(key, 1) – that's a TOCTOU race condition. The easiest solution is to not do anything, and let the user acquire a mutex if desired. A truly safe design would have to anticipate use cases like dict.AddOrCreate(key, 1).
    – amon
    Commented May 7 at 11:08
  • 1
    Beware that liberally adding synchronization primitives (Mutex etc) can be pretty inefficient, and it can also lead to deadlocks when your library invokes callbacks while holding some lock. IMO thread synchronization should usually only happen at the application layer, mostly directly inside the thread function, but not deep in ordinary libraries (except those that implement synchronization primitives of course).
    – Erlkoenig
    Commented May 8 at 10:29
  • @amon collections are poor example, as unsafe collection would corrupt its internal data structures on cross-thread access. In that sense, using a thread-protected collection is still necessary and useful. For example - a method could be used in isolation, using Contains() without Get() to check is a certain condition is reached. Get() that returns Null object or throws is also useful by itself.
    – Basilevs
    Commented May 8 at 12:46
  • 1
    This may not be possible, but if you make everything immutable, you (generally) get free thread safety. Commented May 8 at 16:37

5 Answers 5

24

You should make your library thread-safe, but that does not mean that you should be sprinkling synchronization primitives around your code base.

For the average library, which is not explicitly designed to communicate across threads, they should be thread-safe to the level that different threads can invoke methods on different objects (possibly of the same type) without interfering with each other or getting incorrect results. This is also known as thread-compatibility.

This can be done in several ways, in order of preference:

  1. Avoid using (writable) shared internal state that might be accessed from multiple threads. If a user decides to create an object and share that across threads, then it is the responsibility of the user to ensure proper synchronization.
  2. If it makes sense, put the internal data that gets shared between objects in thread-local storage.
  3. Put synchronization primitives around the access to the shared internal state.

If your library is specifically designed for inter-thread communication, then you quite quickly end up in the last option mentioned above.

6
  • 1
    I don't know whether the terminology varies between platforms, but I'd call that thread-compatibility — for full thread-safety, different threads could safely invoke methods on the same object simultaneously.  (However, thread-compatibility seems to be the most suitable goal for libraries in general, as it allows the caller to implement full thread safety if needed, without requiring its overhead.)
    – gidds
    Commented May 8 at 0:06
  • @gidds: Unfortunately, there is a bit of an infelicity for Rust. The easiest way to solve a wide variety of ownership issues is to wrap something in Rc<RefCell<T>> or Arc<RwLock<T>>, but neither of those are ideal for this purpose. The former is thread-incompatible because it does not synchronize the reference counting and borrow-tracking logic at all, and the latter is fully thread-safe because it locks the pointee. So you pretty much have to decide whether you want to do threads or not if you're going to use this escape hatch.
    – Kevin
    Commented May 8 at 1:56
  • 2
    @gidds, this is the first time I heard about thread-compatibility, but I like the term. Commented May 8 at 6:50
  • @BartvanIngenSchenau You prompted me to check…  It seems I got the term from the 1st edition of Joshua Bloch's excellent book Effective Java.  Its item headed ‘Document Thread Safety’ lists five common cases: Immutable, Thread-safe (mutable but never needs external synchronisation), Conditionally thread-safe (some methods need external sync.), Thread-compatible (needs external sync.), and Thread-hostile (can't be used concurrently even with external sync.).  (contd…)
    – gidds
    Commented May 8 at 22:54
  • …However, although the 2nd and 3rd editions still have that item, with the same five cases, the second one is renamed to Unconditionally thread-safe, and the fourth one to Not thread-safe — so it seems the term ‘thread-compatible’ may not be as widely used as I thought!
    – gidds
    Commented May 8 at 22:54
7

Also, what would your approach when documenting the library? Should we explicitly state the absence of thread-safety or let the consumers assume that it is not thread safe?

It might be useful to take a look at the documentation of some common classes, for example List

Thread Safety

Public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

It is safe to perform multiple read operations on a List, but issues can occur if the collection is modified while it's being read. To ensure thread safety, lock the collection during a read or write operation. To enable a collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization. For collections with built-in synchronization, see the classes in the System.Collections.Concurrent namespace. For an inherently thread-safe alternative, see the ImmutableList class.

This forms the basis of my expectations:

  1. Static members should be thread safe
  2. Objects should not have any special thread affinity. I.e. you can create an object on one thread, and use it from another.
  3. No hidden, unsynchronized, shared state. If shared state is needed, make this explicit in some way, or make sure access is synchronized. I.e. locking the object is sufficient for thread safety.
  4. If the class is named "Concurrent", "Threadsafe", or "Immutable", it should be thread safe.

It is especially important to note any deviations from expectations. But I would recommend to be explicit in the documentation. There is few things worse than having to guess if something is safe or not, and some things might not be obvious, like if a method modifies internal state or not.

UI related objects are one notable exception from these general rules.

I would suggest making a general statement of the threading model used in your library, link to this from any type documentation to make it easier to find, and note any deviations.

1
  • This is a much better advice than the accepted answer of "make your library thread-safe" as doing so could have a lot of trade-offs that can be worse than not having thread-safety. Documentation and proper naming, yet again, is key. Avoid reinventing the wheel, but rather following suit as to what other library makers do, such as what Microsoft does with the BCL, which was beautifully demonstrated in this answer. Commented May 21 at 6:06
3

It is your responsibility to make the library useable and useful for the consumer. It doesn't have to be thread safe, but you need make that very clear in the documentation and explain what the expected usage pattern should be

A common way libraries that are not thread safe for multiple threads accessing a single object is to handle this is by building the library in a way that instantiating a new object per thread is safe (i.e. doesn't share configurations via static variables or disk). In most cases the consumer can trivially handle this by changing to a transient scope on their IoC. The key part is letting the consumer know they need to do this.

1

It depends

I am contributing to a library that is thread safe by default. We decided to do this because making it thread safe by default makes our developer lives easier, reduces client misuse, there is no inherent performance gain by not being thread safe, and making it thread safe from the caller side is much harder while being somewhat trivial on our side. It also doesn't require complicated things like system wide mutex.

If you do go down the thread safe path, be careful to avoid lock statements, as these may cause unexpected behaviors in async methods.

Regarding the MSDN guideline: Microsoft themselves distribute thread safe classes e.g: https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent?view=net-8.0

By thread safe, this means:

  1. No shared state between different instances of the same class.
  2. Accessing the same object from different threads will not make the object's internal state to become corrupt. It may result in unusual behavior, but not result in a crash or otherwise unrecoverable situation.
  3. Threads reading a property of the objects will get the latest written value of that property, when reading and writing occur on different threads. If a 2nd write occurs right after the read, then the read will not see the modified data.
  4. If a particular operation needs to happen on a specific thread (most commonly on the UI thread), the object will automatically handle that situation, even if the call comes from the wrong thread.

However, this only applies to the specifics of this library. My suggestion is to try and go thread safe if your library is likely to be used in a multithreaded environment, at the very least try to prevent corrupted states due to thread races. This will at the minimum save you time trying to educate people on how to avoid crashes caused by multithreading.

5
  • What do you mean by thread safe here? Afaik synchronising modifications to one object over multiple threads is less performant than not synchronising, even if you don't use locks. Do you mean not having hidden shared state between different objects?
    – Caleth
    Commented May 8 at 10:05
  • @Caleth added some clarifications
    – Ccm
    Commented May 8 at 11:56
  • What does "the latest written value" even mean without synchronisation?
    – Caleth
    Commented May 8 at 16:16
  • And if you are doing synchronisation, then you could gain performance by not doing that synchronisation, for the single thread case, so yes, there is a "inherent performance gain by not being thread safe"
    – Caleth
    Commented May 8 at 16:22
  • @Caleth why do you assume there's no synchronisation? Also you overestimate the performance overhead of synchronison. The difference between mutex and no mutex was within the margin of error, less than 1ms. The library is used in real time applications, with hard performance targets of 6ms. Exceeding 6ms leads to... unfortunate scenarios
    – Ccm
    Commented May 8 at 16:35
-1

“Enforce thread safety” is difficult depending on what you are doing.

For example, if two threads read the same data, that should not crash, and give the same data to both threads.

What if one thread tries to read data while the other tries to change it, as close together as possible? You can’t guarantee that the data read is the old of the new data. You can guarantee that it doesn’t crash and is either the old or the new data. But either way it’s a problem. So do you want to make any guarantees?

It’s hard to decide what guarantees you should give.

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