10 ms·

Common Multithreading Mistakes in C# – Unsafe Assumptions

203 points by douche 10y ago

exDM69 10y ago
Nice writeup, and most of this isn't specific to C#, but applies to native code (or Java) as well. These are typical misconceptions of people who have not studied concurrecy. What is specific to C# (applies to Java too) is having an implicit mutex/condition pair in each object. I think this is a terrible design mistake because it's not very practical and it's confusing to newbies (a big stumbling block for students when I studied concurrency at the uni). It's not very practical because in most concurrency tasks I've dealt with (not using C# or Java), there's typically more conditions than there are mutexes (typically 2-4 + O(n) conditions per mutex). In Java/C# land, the typical solution would be to have a complex conditional expression, all the threads spinning on a .wait() there and then over-use of .notifyAll() instead of .notify() causing lots of spurious wakeups and wasting precious cpu cycles. It's confusing because of the reason above, it's easy to go "I'll solve this by adding another mutex". Unfortunately this is seldom the correct solution (to any problem), and a much better result would be achieved adding more condition variables to wait on. I wouldn't mind too much about a design mistake in a language I almost never use (Java/C#) if it wasn't the de facto language for learning about concurrency at universities. This has produced so many engineer with a twisted view on concurrency. I understand that "Java is easy" and "C is hard" but when we're already talking about memory models, multi-core, cache coherency and atomicity, C-the-language isn't the hard part and Java-the-language does very little to help with those parts.
- pvg 10y ago
  In Java/C# land, the typical solution would be to have a complex conditional expression, all the threads spinning on a .wait() there and then over-use of .notifyAll() instead of .notify() causing lots of spurious wakeups and wasting precious cpu cycles. The typical solution is to use the higher-level constructs provided by the standard library that take care of these details for you. In Java, at least, explicit mutexes, notify, etc in application code are roughly the equivalent of typing 'AES'. I'm somewhat surprised this C# article happily tells you to just keep adding locks until you think things work. "I mean if you have a hunchback, just throw a little glitter on it, honey, and go dancing."
  - zzzcpan 10y ago
    It's more complicated than that. Implicit synchronization and multithreading in libraries is error-prone, poorly performant, has a significant cognitive overhead. It's a liability if you are chasing performance and a liability if you aren't. So to be good citizens libraries have to assume that a user might need a complete control in choosing and using a concurrency model (of course unless they are the ones providing concurrency models themselves).
    - pvg 10y ago
      I'm talking about libraries that provide higher-level concurrency constructs not implicit multithreading in a library that does, say, image manipulation. The whole point of the former is to reduce cognitive load and errors.
      - zzzcpan 10y ago
        I assumed you were talking about things like concurrent queues, etc. They do implicit synchronization and enforce a specific concurrency model, so they should not be used by libraries.
        m_fayer 10y ago
        Yes, but I think he also meant things like the TPL, PLINQ, TPL Dataflow, RX, Akka.NET and so on. I have been happily using one or more of these to write tons of event driven and/or parallel client-side code and haven't touched a lock in years. And libraries exposing APIs through observables is a great idea.
        eropple 10y ago
        I don't dislike TPL and PLINQ (though I don't much care for Akka.NET at all), but at the same time I pretty regularly write code with locks. There are problems for which the simplest solution is to treat it stupidly and imperatively and to run those threads side-by-side with well-defined critical sections.
- vvanders 10y ago
  Another downside of the "throw another mutex at it" is if you're caring about performance(which you probably are) ti's a great way to introduce overhead via death-by-a-thousand-cuts. Each mutex you take has a not-trivial cost but doesn't show up well on a profiler since the individual call is small and spread across an invocation path.
- deleted 10y ago
  [deleted]
jsingleton 10y ago
Nice post. Simply written and easy to understand. There are two others: - http://benbowen.blog/post/cmmics_i/ http://benbowen.blog/post/cmmics_i/ - http://benbowen.blog/post/cmmics_ii/ http://benbowen.blog/post/cmmics_ii/ It's well worth learning about the Interlocked class if you want to do parallel programming. If you get it wrong then you will see incorrect/corrupt results. It's also worth keeping in mind that for simple tasks it can be quicker to do them single threaded, due to the overheads involved. I demonstrated this in a simple benchmark app [0] that I wrote for a chapter of my book on ASP.NET Core [1]. [0]: https://github.com/PacktPublishing/ASP.NET-Core-1.0-High-Performance/blob/master/Chapter%206/ParallelBenchmarking/ParallelBenchmarking/src/ParallelBenchmarking/Program.cs https://github.com/PacktPublishing/ASP.NET-Core-1.0-High-Per... [1]: https://unop.uk/book/ https://unop.uk/book/
cm2187 10y ago
I learned something about Thread.MemoryBarrier(). I expected the following to be safe: Dim V(99) As Integer Parallel.For(0, 100, Sub(i) V(i) = i End Sub) Dim Result = V.Sum If I understand correctly it looks like I need to flush the memory before accessing V: Dim V(99) As Integer Parallel.For(0, 100, Sub(i) V(i) = i End Sub) Threading.Thread.MemoryBarrier() Dim Result = V.Sum
- Matthias247 10y ago
  That's interesting, I had thought the first one to be safe since Parallel.For should already imply synchronization: The operation should only return once everything was done - and the waiting everything done part of it should act as a memory barrier.
- taspeotis 10y ago
  The CLR has some implicit memory barriers that make most things "just work." Take a look at "[t]he following implicitly generate full fences" here: http://www.albahari.com/threading/part4.aspx http://www.albahari.com/threading/part4.aspx. Parallel.For would probably be covered by "[a]nything that relies on signaling." There's more info scattered about in some Stack Overflow answers. The content is useful, just unofficial. E.g. http://stackoverflow.com/a/6932271/242520 http://stackoverflow.com/a/6932271/242520, http://stackoverflow.com/a/681872/242520 http://stackoverflow.com/a/681872/242520
sharpercoder 10y ago
The more threading code I have seen, the more convinced I become that it should be avoided at all cost. The pitfalls are nonobvious and potentially incurring datacorruption. Rather, they should be behind understandable abstractions (like await/async and Task/Task<T>, but those have their own pitfalls) where applicable. Also, I can see value in an "unsafe" block (e.g. a "threading {}" keyword) where the writer of code communicates to the reader "inspect this with special threading awareness"
- ro_sharp 10y ago
  I concur, with a caveat - feel free to use threading / async I/O if you think you really understand the runtime behaviour of your code. I've seen so much code that just runs TaskFactory.StartNew some arbitrary number of times in a loop to run some CPU intensive task with no/effectively no I/O, presupposing a performance improvement as a result. Understand runtime performance. Understand the different failure modes inherent in multi-threaded approaches, and lastly: test, profile, test and profile again!
  - thomasz 10y ago
    Is there something wrong with for(int i = 0; i < Environment.ProcessorCount; i++) Task.Factory.StartNew(...); ?
    - quinnftw 10y ago
      Potentially, yes. Assuming that you are correctly synchronizing the actions inside each Task as to avoid race conditions (a big assumption), there is no control on the number of threads here. What if your program is already using a bunch of threads for something else, and then you go ahead and spawn off 10 more? You will oversaturate the CPU and end up with virtual threads, thus incurring scheduling overhead. In my opinion when you are doing this type of thing you should almost always use a thread pool to ensure that you aren't creating an unreasonable number of threads.
    - mdpopescu 10y ago
      For one thing, prefer Task.Run to Task.Factory.StartNew - see https://blogs.msdn.microsoft.com/pfxteam/2011/10/24/task-run-vs-task-factory-startnew/ https://blogs.msdn.microsoft.com/pfxteam/2011/10/24/task-run...
Pigo 10y ago
I've often wondered what the real gains are for using async/await methods inside a web api call per say, especially when it's a single task being performed, like a query. I've tried to setup concurrent queries, and do whenall or parallel.foreach. But I think I always hit a road block because each query has to be from a separate context for that to work.
- kaliku2 10y ago
  while the awaitable task is .. well.. waiting for the query, the thread servicing it is returned to the thread pool to be reused. This helps with thread pool starvation in the case when the process must service many requests.
  - SideburnsOfDoom 10y ago
    Right. Or you can phrase this as: using async ... await can't make you wait faster. Waiting will take just as long, but it will be more efficient. The thread is freed up to do other things during the wait, increasing the possible throughput.
    - jrs235 10y ago
      So, (is it safe to say?) using await/async will likely be slower for each given awaited operation since there is overhead in thread management and because when the awaited operation finishes it must wait for a thread to become available to continue onward.
      - SideburnsOfDoom 10y ago
        There is some overhead in thread management, yes. But in a server scenario (e.g. a web api) there is already thread management happening, as it all runs in threads from a thread pool. I don't believe that you pay a significant additional cost per await - i.e. if you have one await in your code, then you might as well use as many as you can to chop the operation up into small parts so that operations can flow through better. If you "must wait for a thread to become available" for any significant amount of time then you have run out of threads despite being async, and you have other problems. This article talks about the mechanics of how the operation's completion bubbles back up to your code, in some detail: http://blog.stephencleary.com/2013/11/there-is-no-thread.html http://blog.stephencleary.com/2013/11/there-is-no-thread.htm... See also, avoiding some basic mistakes: http://www.anthonysteele.co.uk/AsyncBasicMistakes http://www.anthonysteele.co.uk/AsyncBasicMistakes in UI code, there is a guideline that "operations that could take over 50ms should be async" (1) so as to not lock the UI thread. But it's clear that this does not apply to web server code where it's all thread pool threads already. 1) http://blog.stephencleary.com/2013/04/ui-guidelines-for-async.html http://blog.stephencleary.com/2013/04/ui-guidelines-for-asyn...
mathw 10y ago
I'd like some more explanation of why the author thinks it's better to use a lock statement than an Interlocked method most of the time. This is really just fundamental concurrency stuff though. Something which is sadly in short supply in some people's skill sets, but then I guess not everyone spent four years working on massively multithreaded C++ software early in their career like I did. I'd really prefer it if C# made you share memory explicitly - default shared memory concurrency is just asking for trouble, in this and many other languages, because you have to do extra to do things right, rather than extra to do things wrong.
- gwbas1c 10y ago
  Yeah, I tend to prefer various forms of Interlocked to avoid deadlocks that happen 0.01% of the time.
- achr2 10y ago
  I agree about the use of Interlocked. Obviously if you already need a lock around additional code, there is no need to use Interlocked, but for any single statements where Interlocked is available, it is a simple performance gain. That said I have seen several incorrect and unnecessary `CompareExchange` usages that could have been avoided with a simple lock...
  - Sharlin 10y ago
    > That said I have seen several incorrect and unnecessary `CompareExchange` usages that could have been avoided with a simple lock... This is, obviously, the answer to the GP's question.
    - TillE 10y ago
      That's no reason for avoiding atomics in simple use cases. "Use locks and then profile" is fairly bad advice. Forget the profiler and always use atomics in simple situations where you know exactly how they work. Just don't try anything complicated with them.
  - blahfuk 10y ago
gnur 10y ago
I know there is a website that has gamified certain race conditions. There was a interface that allowed you to step through code in 2 "threads" and the goal was to defeat the enemy army though deadlocking their code. Does anyone know where it is? It was very informative..
- ttrunck 10y ago
  I know this one: https://deadlockempire.github.io/#menu https://deadlockempire.github.io/#menu
achr2 10y ago
It's odd to have a whole section on coherency and never mention the built-in `volatile` keyword who's entire purpose is to indicate that a variable requires fresh reads..
- squeaky-clean 10y ago
  > In this case, the correct solution is to mark field _A as volatile. If that’s done, the compiler shouldn’t reorder the reads of _A and _B, because the load of _A has load-acquire semantics. However, I should point out that the .NET Framework, through version 4, doesn’t handle this case correctly and, in fact, marking the _A field as volatile will not prevent the read reordering. This issue has been fixed in the .NET Framework version 4.5. From the article linked where they mention volatile and the C# memory model. Seems like volatile may not actually have worked as intended for this case!
  - achr2 10y ago
    I'm not saying volatile is a good solution, but to not cover it in this article is an odd omission - especially since he shows an explicit memory barrier.
jimmaswell 10y ago
Not working very well in mobile: https://imgur.com/xPn8YLc https://imgur.com/xPn8YLc A simple table would have worked fine.
sidlls 10y ago
These are common mistakes novices to multithreaded programming make, presented and explained well. Very nice. In my experience another mistake made by novices and journeymen alike (at least occasionally by the latter) is to reach for this tool without careful consideration of whether it's even necessary.
- bbcbasic 10y ago
  I'm such a novice I avoid reaching for the tool ;)
novaleaf 10y ago
the biggest gotcha I've seen with C# multithreaded is the usage of the basic .NET Collections: Dictionary<K,V> and List<V> they will mostly work (like multiple threads reading, or 1 writes while another retrieves), except when two threads try to simultaneously write, or when 1 writes while another enumerates. When that happens, all bets are off, and you will silently get obtuse errors such as null return values or enumerating past the end of the collection, or a corrupt collection that throws exceptions from other (seemingly) random and unrelated areas. I believe the new concurrent collections take care of this, but it is still so easy for beginners to shoot themselves with async programming. very much in stark contrast to how dev-friendly the rest of the CLR is.
- dahauns 10y ago
  New concurrent collections? .NET 4 is over eight years old now. And for all these years, the docs have explicitly informed about lack of thread safety in the non-concurrent versions and referenced the concurrent equivalents.
  - novaleaf 10y ago
    yes, but it doens't stop novices from doing it, or perhaps if you have existing code you are trying to parallelize.
- lmm 10y ago
  That's pretty unfortunate by comparison with Java, where while ConcurrentModificationException is specified as unreliable you tend to hit it pretty quickly in practice (and then hopefully the docs encourage you to fix your code properly rather than catching it).
  - alkonaut 10y ago
    You get that in C# as well, if you modify a collection that is being enumerated. That is simply thrown by e.g the List enumerator MoveNext() method if it discovers that the underlying list has been modified, which it detects since all mutating methods increments a version. You don't even need multiple threads for this to occur! You can just create a list enumerator, and then accidentally add/remove an item while iterating. The more difficult problems to detect are those unrelated to collection versions + enumerators. For example accidentally reading a Dictionary<K, V> while someone is adding to it on another thread (this requires true concurrency). This one has no builtin detection for modification. Does the Java dictionary really check thread identifiers for who is modifying? Or use some kind of entry counter to detect concurrency? I'd love to have that in debug builds in C#. The result in C# is usually that it works 99% of the time and corrupts silently in the edge case that someone happened to read the hash table at the same time as an internal array was resized or similar. Typical consequence is that it reports a count of N and Contains(x) is true, but when listing the contents we see some other number of items, and no sign of x... It's that customer report with a stack trace you say "that's impossible, foo is never null there". Or that unit test that only fails one build of 1000 and you blame cosmic rays first...
Pxtl 10y ago
The one that caught me recently was assuming that you could use a Dictionary as a concurrent cache. You can't. Dictionary can be read concurrently, but once you start writing to it concurrently all bets are off. ConcurrentDictionary implements IDictionary and allows concurrent writes. Also, Entity Framework. We had an icky bug coming from somebody storing the datacontext in a member variable. Don't do that.
- cm2187 10y ago
  The one I am concerned about is iterators (for each). I have read somewhere that iterators are stateful and I should be careful when using them in a multi-threaded environment. Currently I am ignoring this advice and use them happily (on static lists and dictionaries) but I am not sure if this isn't going to bite me some day. But I don't understand how it would bite me.
  - jdmichal 10y ago
    That is because your different threads are not using the same `Enumerator`. I'm guessing your threads look something like this, based on your mention of `foreach`: // In multiple threads: foreach(element in collection) { ... } `collection` is not an `Enumerator` but rather an `Enumerable`. The `foreach` statement calls `collection.GetEnumerator`, so each thread gets its own iterator. You would only see inconsistencies if another thread modified the collection during iteration. Try this instead, and you'll guaranteed see issues: // Before: IEnumerator enumerator = collection.getEnumerator(); // In multiple threads: while(enumerator.Move()) { var element = enumerator.Current; }
- tracker1 10y ago
  You want ConcurrentDictionary[1]... the worst one I saw was attaching information keyed off the thread id. Note: in ASP.Net lifecycle there are no guarantees that two events (load, prerender, render, etc) will be performed on the same thread from the pool. Wound up seeing some REALLY weird issues... HttpContext.Current.Items is your friend there. https://msdn.microsoft.com/en-us/library/dd287191(v=vs.110).aspx https://msdn.microsoft.com/en-us/library/dd287191(v=vs.110)....
samfisher83 10y ago
I think I learned this during learning Peterson's algorithm. The professor emphasized the flag had to be a single operation variable.
jorgeleo 10y ago
Interesting is also that these recommendations also apply to the use of the async/await pair
- mihular 10y ago
  Only when running code in different threads. async/await isn't necessarily multithreaded. Actually usually isn't.
  - jorgeleo 10y ago
    Which makes it worse because it opens up the possibility of intermitent bugs, and also those "it works on my machine, I don't know why it does not work on the server" So it is better to treat them as multithreaded always... which falls back on the original post.
manigandham 10y ago
Always use Interlocked over a lock statement if you're just dealing with incrementing numbers. It's 1-line, faster and always safe. Author is mistaken for advocating a lock in these situations. Only need lock() and the full Monitor classes if there's more to be done within the locked statement.
ibgib 10y ago
I enjoyed this article. I started concurrency with Delphi 5 using critical sections, mutexes, semaphores, etc. Then I started using C# which was a major upgrade, and I loved having the simplicity of things like the `BackgroundWorker` class and the `lock` statement which certainly beat the need to write my own multi-threaded class for every little thing in Delphi. I recall some of article's mentioned atomicity problems, and I would have to juggle when to use Interlocked vs locks, etc. For example, it mentions floats and hardware-specific issues with 64-bit ints, but from what I recall I treated booleans as atomic and not needing Interlocked or locks, but floats I did. That CPU caching detail level though sounds mighty insidious. I also forged ahead and used TPL, async/await, Rx (Observables are pretty awesome)... But now I live on Erlang's BEAM (via Elixir) and I freaking love it. The real gain for me is that I found I didn't need mutexes, locks, critical sections, etc., because the super lightweight thread-like Erlang processes (not OS processes) themselves run in parallel but each one runs in a single-threaded manner. This effectively turns each process itself into its own critical section, and it's this aspect that I personally have found extremely valuable.
- bonzini 10y ago
  Do your Erlang processes never execute an asynchronous (await-like) operation? If they did, any shared data would need to be protected by a mutex just as like as with multiple OS threads.
  - ibgib 10y ago
    > Do your Erlang processes never execute an asynchronous (await-like) operation? If they did, any shared data would need to be protected by a mutex just as like as with multiple OS threads. I think perhaps what you are getting at is that there is still need for coordination for parallel processes, and that is totally correct. But each BEAM(!) process has its own non-shared memory and runs on a single thread. This is my point in that it doesn't need the mutex because the process' execution is its own critical section. I've structured my ibGib engine to be more functional and the need for the mutex/critical sections has disappeared for me. For example, check out this SO post that I just googled: http://stackoverflow.com/questions/28554114/applying-a-mutex-into-an-erlang-example http://stackoverflow.com/questions/28554114/applying-a-mutex... In the answer, he tells the OP (who thinks he needs a mutex) that what he can actually do is just do the work in the process. This kind of understanding of single-threaded "bounded context"-like execution, combined with ubiquitous immutability has been massively helpful for me with ibGib. In C# for example, I was trying to apply `Task<ib>` all over the place (yes it is non-idiomatic lowercasing of the interface...totally aware of this sin in C#). And so even with async/await awesomeness, the code is just riddled with async/await, blah blah blah. The same goes for using Rx in C# for a "microservices" engine (I started it before that was a fad and I called them autonomous services lol) and my more recent POC with Rxjs in TypeScript/JavaScript. The point is the whole approach of concurrency and whatnot are "addons" in the form of mutexes, semaphores, critical sections, locks, barriers (I've used quite a few over the years). But with programming on the BEAM(!) with Elixir and Erlang, parallel coding is just a more natural experience, because it's just how it's done. In my view, this largely stems from the aspect I mentioned: that they have parallel processes, each running its own memory and on a single thread, communicating with message passing (now called the Actor Model but they didn't know that at the time). On top of this fundamental design decision, they built up an awesome infrastructure with OTP, Supervision trees, and more. But anyway, I didn't mean to write that much - but it's just been a delightful experience, since my very first application in Delphi was a transcription app with a from-scratch multi-threaded realtime document checker (like a spell checker, but more pluggable and geared towards transcription with text expansion which is a totally different use case than existing off-the-shelf spell checkers).
jaegerpicker 10y ago
It's a cool series. Mostly review for me but I certainly passed it to my team to review. Ultimately I say that 99% of the time the correct answer is to not use raw threads and never share memory. There are much better concurrency and parallel patterns out there. Akka(.net in this case), Coroutines and Channels, Erlang Actors, Clojure's core.async, .net's async/await even. Honestly if you are reaching for threads you probably should ask yourself if some other way would be better and safer.
TazeTSchnitzel 10y ago
“Newbie” question: if reading a value without having a lock on it can give you a torn read, wouldn't it be possible to crash the CLR by getting a torn read on a reference? Or, does it account for this somehow?
- Eyas 10y ago
  A reference fits perfectly in a word-sized chunk. A pointer/reference is typically 32 or 64-bits wide, since it is simply pointing to a location in memory.
- dsp1234 10y ago
  from the ECMA-335 standard (which is not the latest reference, but is easily accessible): "A conforming CLI shall guarantee that read and write access to properly aligned memory locations no larger than the native word size (the size of type native int) is atomic (see §I.12.6.2) when all the write accesses to a location are the same size." Since references are the native word size, the CLR will ensure that there is no tearing. Note that it is possible to produce a torn read variables that are larger than a native-int, even of native types such as decimal[1]. [0] - http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf http://www.ecma-international.org/publications/files/ECMA-ST... section I.12.6.6, page 102 [1] - http://stackoverflow.com/questions/23262513/reproduce-torn-reads-of-decimal-in-c-sharp http://stackoverflow.com/questions/23262513/reproduce-torn-r...
sundvor 10y ago
Great article. I learned a lot from reading this; being relatively new to C#, now I certainly know what to look for when coming across multithreading.

203 points by douche 10y ago

Nice writeup, and most of this isn't specific to C#, but applies to native code (or Java) as well. These are typical misconceptions of people who have not studied concurrecy. What is specific to C# (applies to Java too) is having an implicit mutex/condition pair in each object. I think this is a terrible design mistake because it's not very practical and it's confusing to newbies (a big stumbling block for students when I studied concurrency at the uni). It's not very practical because in most concurrency tasks I've dealt with (not using C# or Java), there's typically more conditions than there are mutexes (typically 2-4 + O(n) conditions per mutex). In Java/C# land, the typical solution would be to have a complex conditional expression, all the threads spinning on a .wait() there and then over-use of .notifyAll() instead of .notify() causing lots of spurious wakeups and wasting precious cpu cycles. It's confusing because of the reason above, it's easy to go "I'll solve this by adding another mutex". Unfortunately this is seldom the correct solution (to any problem), and a much better result would be achieved adding more condition variables to wait on. I wouldn't mind too much about a design mistake in a language I almost never use (Java/C#) if it wasn't the de facto language for learning about concurrency at universities. This has produced so many engineer with a twisted view on concurrency. I understand that "Java is easy" and "C is hard" but when we're already talking about memory models, multi-core, cache coherency and atomicity, C-the-language isn't the hard part and Java-the-language does very little to help with those parts.

pvg 10y ago
In Java/C# land, the typical solution would be to have a complex conditional expression, all the threads spinning on a .wait() there and then over-use of .notifyAll() instead of .notify() causing lots of spurious wakeups and wasting precious cpu cycles. The typical solution is to use the higher-level constructs provided by the standard library that take care of these details for you. In Java, at least, explicit mutexes, notify, etc in application code are roughly the equivalent of typing 'AES'. I'm somewhat surprised this C# article happily tells you to just keep adding locks until you think things work. "I mean if you have a hunchback, just throw a little glitter on it, honey, and go dancing."
- zzzcpan 10y ago
  It's more complicated than that. Implicit synchronization and multithreading in libraries is error-prone, poorly performant, has a significant cognitive overhead. It's a liability if you are chasing performance and a liability if you aren't. So to be good citizens libraries have to assume that a user might need a complete control in choosing and using a concurrency model (of course unless they are the ones providing concurrency models themselves).
  - pvg 10y ago
    I'm talking about libraries that provide higher-level concurrency constructs not implicit multithreading in a library that does, say, image manipulation. The whole point of the former is to reduce cognitive load and errors.
    - zzzcpan 10y ago
      I assumed you were talking about things like concurrent queues, etc. They do implicit synchronization and enforce a specific concurrency model, so they should not be used by libraries.
      - m_fayer 10y ago
        Yes, but I think he also meant things like the TPL, PLINQ, TPL Dataflow, RX, Akka.NET and so on. I have been happily using one or more of these to write tons of event driven and/or parallel client-side code and haven't touched a lock in years. And libraries exposing APIs through observables is a great idea.
        eropple 10y ago
        I don't dislike TPL and PLINQ (though I don't much care for Akka.NET at all), but at the same time I pretty regularly write code with locks. There are problems for which the simplest solution is to treat it stupidly and imperatively and to run those threads side-by-side with well-defined critical sections.
vvanders 10y ago
Another downside of the "throw another mutex at it" is if you're caring about performance(which you probably are) ti's a great way to introduce overhead via death-by-a-thousand-cuts. Each mutex you take has a not-trivial cost but doesn't show up well on a profiler since the individual call is small and spread across an invocation path.
deleted 10y ago
[deleted]

jsingleton 10y ago

Nice post. Simply written and easy to understand. There are two others: - http://benbowen.blog/post/cmmics_i/ http://benbowen.blog/post/cmmics_i/ - http://benbowen.blog/post/cmmics_ii/ http://benbowen.blog/post/cmmics_ii/ It's well worth learning about the Interlocked class if you want to do parallel programming. If you get it wrong then you will see incorrect/corrupt results. It's also worth keeping in mind that for simple tasks it can be quicker to do them single threaded, due to the overheads involved. I demonstrated this in a simple benchmark app [0] that I wrote for a chapter of my book on ASP.NET Core [1]. [0]: https://github.com/PacktPublishing/ASP.NET-Core-1.0-High-Performance/blob/master/Chapter%206/ParallelBenchmarking/ParallelBenchmarking/src/ParallelBenchmarking/Program.cs https://github.com/PacktPublishing/ASP.NET-Core-1.0-High-Per... [1]: https://unop.uk/book/ https://unop.uk/book/

cm2187 10y ago

I learned something about Thread.MemoryBarrier(). I expected the following to be safe: Dim V(99) As Integer Parallel.For(0, 100, Sub(i) V(i) = i End Sub) Dim Result = V.Sum If I understand correctly it looks like I need to flush the memory before accessing V: Dim V(99) As Integer Parallel.For(0, 100, Sub(i) V(i) = i End Sub) Threading.Thread.MemoryBarrier() Dim Result = V.Sum

Matthias247 10y ago
That's interesting, I had thought the first one to be safe since Parallel.For should already imply synchronization: The operation should only return once everything was done - and the waiting everything done part of it should act as a memory barrier.
taspeotis 10y ago
The CLR has some implicit memory barriers that make most things "just work." Take a look at "[t]he following implicitly generate full fences" here: http://www.albahari.com/threading/part4.aspx http://www.albahari.com/threading/part4.aspx. Parallel.For would probably be covered by "[a]nything that relies on signaling." There's more info scattered about in some Stack Overflow answers. The content is useful, just unofficial. E.g. http://stackoverflow.com/a/6932271/242520 http://stackoverflow.com/a/6932271/242520, http://stackoverflow.com/a/681872/242520 http://stackoverflow.com/a/681872/242520

sharpercoder 10y ago

The more threading code I have seen, the more convinced I become that it should be avoided at all cost. The pitfalls are nonobvious and potentially incurring datacorruption. Rather, they should be behind understandable abstractions (like await/async and Task/Task<T>, but those have their own pitfalls) where applicable. Also, I can see value in an "unsafe" block (e.g. a "threading {}" keyword) where the writer of code communicates to the reader "inspect this with special threading awareness"

ro_sharp 10y ago
I concur, with a caveat - feel free to use threading / async I/O if you think you really understand the runtime behaviour of your code. I've seen so much code that just runs TaskFactory.StartNew some arbitrary number of times in a loop to run some CPU intensive task with no/effectively no I/O, presupposing a performance improvement as a result. Understand runtime performance. Understand the different failure modes inherent in multi-threaded approaches, and lastly: test, profile, test and profile again!
- thomasz 10y ago
  Is there something wrong with for(int i = 0; i < Environment.ProcessorCount; i++) Task.Factory.StartNew(...); ?
  - quinnftw 10y ago
    Potentially, yes. Assuming that you are correctly synchronizing the actions inside each Task as to avoid race conditions (a big assumption), there is no control on the number of threads here. What if your program is already using a bunch of threads for something else, and then you go ahead and spawn off 10 more? You will oversaturate the CPU and end up with virtual threads, thus incurring scheduling overhead. In my opinion when you are doing this type of thing you should almost always use a thread pool to ensure that you aren't creating an unreasonable number of threads.
  - mdpopescu 10y ago
    For one thing, prefer Task.Run to Task.Factory.StartNew - see https://blogs.msdn.microsoft.com/pfxteam/2011/10/24/task-run-vs-task-factory-startnew/ https://blogs.msdn.microsoft.com/pfxteam/2011/10/24/task-run...

Pigo 10y ago

I've often wondered what the real gains are for using async/await methods inside a web api call per say, especially when it's a single task being performed, like a query. I've tried to setup concurrent queries, and do whenall or parallel.foreach. But I think I always hit a road block because each query has to be from a separate context for that to work.

kaliku2 10y ago
while the awaitable task is .. well.. waiting for the query, the thread servicing it is returned to the thread pool to be reused. This helps with thread pool starvation in the case when the process must service many requests.
- SideburnsOfDoom 10y ago
  Right. Or you can phrase this as: using async ... await can't make you wait faster. Waiting will take just as long, but it will be more efficient. The thread is freed up to do other things during the wait, increasing the possible throughput.
  - jrs235 10y ago
    So, (is it safe to say?) using await/async will likely be slower for each given awaited operation since there is overhead in thread management and because when the awaited operation finishes it must wait for a thread to become available to continue onward.
    - SideburnsOfDoom 10y ago
      There is some overhead in thread management, yes. But in a server scenario (e.g. a web api) there is already thread management happening, as it all runs in threads from a thread pool. I don't believe that you pay a significant additional cost per await - i.e. if you have one await in your code, then you might as well use as many as you can to chop the operation up into small parts so that operations can flow through better. If you "must wait for a thread to become available" for any significant amount of time then you have run out of threads despite being async, and you have other problems. This article talks about the mechanics of how the operation's completion bubbles back up to your code, in some detail: http://blog.stephencleary.com/2013/11/there-is-no-thread.html http://blog.stephencleary.com/2013/11/there-is-no-thread.htm... See also, avoiding some basic mistakes: http://www.anthonysteele.co.uk/AsyncBasicMistakes http://www.anthonysteele.co.uk/AsyncBasicMistakes in UI code, there is a guideline that "operations that could take over 50ms should be async" (1) so as to not lock the UI thread. But it's clear that this does not apply to web server code where it's all thread pool threads already. 1) http://blog.stephencleary.com/2013/04/ui-guidelines-for-async.html http://blog.stephencleary.com/2013/04/ui-guidelines-for-asyn...

mathw 10y ago

I'd like some more explanation of why the author thinks it's better to use a lock statement than an Interlocked method most of the time. This is really just fundamental concurrency stuff though. Something which is sadly in short supply in some people's skill sets, but then I guess not everyone spent four years working on massively multithreaded C++ software early in their career like I did. I'd really prefer it if C# made you share memory explicitly - default shared memory concurrency is just asking for trouble, in this and many other languages, because you have to do extra to do things right, rather than extra to do things wrong.

gwbas1c 10y ago
Yeah, I tend to prefer various forms of Interlocked to avoid deadlocks that happen 0.01% of the time.
achr2 10y ago
I agree about the use of Interlocked. Obviously if you already need a lock around additional code, there is no need to use Interlocked, but for any single statements where Interlocked is available, it is a simple performance gain. That said I have seen several incorrect and unnecessary `CompareExchange` usages that could have been avoided with a simple lock...
- Sharlin 10y ago
  > That said I have seen several incorrect and unnecessary `CompareExchange` usages that could have been avoided with a simple lock... This is, obviously, the answer to the GP's question.
  - TillE 10y ago
    That's no reason for avoiding atomics in simple use cases. "Use locks and then profile" is fairly bad advice. Forget the profiler and always use atomics in simple situations where you know exactly how they work. Just don't try anything complicated with them.
- blahfuk 10y ago

gnur 10y ago

I know there is a website that has gamified certain race conditions. There was a interface that allowed you to step through code in 2 "threads" and the goal was to defeat the enemy army though deadlocking their code. Does anyone know where it is? It was very informative..

ttrunck 10y ago
I know this one: https://deadlockempire.github.io/#menu https://deadlockempire.github.io/#menu

achr2 10y ago

It's odd to have a whole section on coherency and never mention the built-in `volatile` keyword who's entire purpose is to indicate that a variable requires fresh reads..

squeaky-clean 10y ago
> In this case, the correct solution is to mark field _A as volatile. If that’s done, the compiler shouldn’t reorder the reads of _A and _B, because the load of _A has load-acquire semantics. However, I should point out that the .NET Framework, through version 4, doesn’t handle this case correctly and, in fact, marking the _A field as volatile will not prevent the read reordering. This issue has been fixed in the .NET Framework version 4.5. From the article linked where they mention volatile and the C# memory model. Seems like volatile may not actually have worked as intended for this case!
- achr2 10y ago
  I'm not saying volatile is a good solution, but to not cover it in this article is an odd omission - especially since he shows an explicit memory barrier.

jimmaswell 10y ago

Not working very well in mobile: https://imgur.com/xPn8YLc https://imgur.com/xPn8YLc A simple table would have worked fine.

sidlls 10y ago

These are common mistakes novices to multithreaded programming make, presented and explained well. Very nice. In my experience another mistake made by novices and journeymen alike (at least occasionally by the latter) is to reach for this tool without careful consideration of whether it's even necessary.

bbcbasic 10y ago
I'm such a novice I avoid reaching for the tool ;)

novaleaf 10y ago

the biggest gotcha I've seen with C# multithreaded is the usage of the basic .NET Collections: Dictionary<K,V> and List<V> they will mostly work (like multiple threads reading, or 1 writes while another retrieves), except when two threads try to simultaneously write, or when 1 writes while another enumerates. When that happens, all bets are off, and you will silently get obtuse errors such as null return values or enumerating past the end of the collection, or a corrupt collection that throws exceptions from other (seemingly) random and unrelated areas. I believe the new concurrent collections take care of this, but it is still so easy for beginners to shoot themselves with async programming. very much in stark contrast to how dev-friendly the rest of the CLR is.

dahauns 10y ago
New concurrent collections? .NET 4 is over eight years old now. And for all these years, the docs have explicitly informed about lack of thread safety in the non-concurrent versions and referenced the concurrent equivalents.
- novaleaf 10y ago
  yes, but it doens't stop novices from doing it, or perhaps if you have existing code you are trying to parallelize.
lmm 10y ago
That's pretty unfortunate by comparison with Java, where while ConcurrentModificationException is specified as unreliable you tend to hit it pretty quickly in practice (and then hopefully the docs encourage you to fix your code properly rather than catching it).
- alkonaut 10y ago
  You get that in C# as well, if you modify a collection that is being enumerated. That is simply thrown by e.g the List enumerator MoveNext() method if it discovers that the underlying list has been modified, which it detects since all mutating methods increments a version. You don't even need multiple threads for this to occur! You can just create a list enumerator, and then accidentally add/remove an item while iterating. The more difficult problems to detect are those unrelated to collection versions + enumerators. For example accidentally reading a Dictionary<K, V> while someone is adding to it on another thread (this requires true concurrency). This one has no builtin detection for modification. Does the Java dictionary really check thread identifiers for who is modifying? Or use some kind of entry counter to detect concurrency? I'd love to have that in debug builds in C#. The result in C# is usually that it works 99% of the time and corrupts silently in the edge case that someone happened to read the hash table at the same time as an internal array was resized or similar. Typical consequence is that it reports a count of N and Contains(x) is true, but when listing the contents we see some other number of items, and no sign of x... It's that customer report with a stack trace you say "that's impossible, foo is never null there". Or that unit test that only fails one build of 1000 and you blame cosmic rays first...

Pxtl 10y ago

The one that caught me recently was assuming that you could use a Dictionary as a concurrent cache. You can't. Dictionary can be read concurrently, but once you start writing to it concurrently all bets are off. ConcurrentDictionary implements IDictionary and allows concurrent writes. Also, Entity Framework. We had an icky bug coming from somebody storing the datacontext in a member variable. Don't do that.

cm2187 10y ago
The one I am concerned about is iterators (for each). I have read somewhere that iterators are stateful and I should be careful when using them in a multi-threaded environment. Currently I am ignoring this advice and use them happily (on static lists and dictionaries) but I am not sure if this isn't going to bite me some day. But I don't understand how it would bite me.
- jdmichal 10y ago
  That is because your different threads are not using the same `Enumerator`. I'm guessing your threads look something like this, based on your mention of `foreach`: // In multiple threads: foreach(element in collection) { ... } `collection` is not an `Enumerator` but rather an `Enumerable`. The `foreach` statement calls `collection.GetEnumerator`, so each thread gets its own iterator. You would only see inconsistencies if another thread modified the collection during iteration. Try this instead, and you'll guaranteed see issues: // Before: IEnumerator enumerator = collection.getEnumerator(); // In multiple threads: while(enumerator.Move()) { var element = enumerator.Current; }
tracker1 10y ago
You want ConcurrentDictionary[1]... the worst one I saw was attaching information keyed off the thread id. Note: in ASP.Net lifecycle there are no guarantees that two events (load, prerender, render, etc) will be performed on the same thread from the pool. Wound up seeing some REALLY weird issues... HttpContext.Current.Items is your friend there. https://msdn.microsoft.com/en-us/library/dd287191(v=vs.110).aspx https://msdn.microsoft.com/en-us/library/dd287191(v=vs.110)....

samfisher83 10y ago

I think I learned this during learning Peterson's algorithm. The professor emphasized the flag had to be a single operation variable.

jorgeleo 10y ago

Interesting is also that these recommendations also apply to the use of the async/await pair

mihular 10y ago
Only when running code in different threads. async/await isn't necessarily multithreaded. Actually usually isn't.
- jorgeleo 10y ago
  Which makes it worse because it opens up the possibility of intermitent bugs, and also those "it works on my machine, I don't know why it does not work on the server" So it is better to treat them as multithreaded always... which falls back on the original post.

manigandham 10y ago

Always use Interlocked over a lock statement if you're just dealing with incrementing numbers. It's 1-line, faster and always safe. Author is mistaken for advocating a lock in these situations. Only need lock() and the full Monitor classes if there's more to be done within the locked statement.

ibgib 10y ago

I enjoyed this article. I started concurrency with Delphi 5 using critical sections, mutexes, semaphores, etc. Then I started using C# which was a major upgrade, and I loved having the simplicity of things like the `BackgroundWorker` class and the `lock` statement which certainly beat the need to write my own multi-threaded class for every little thing in Delphi. I recall some of article's mentioned atomicity problems, and I would have to juggle when to use Interlocked vs locks, etc. For example, it mentions floats and hardware-specific issues with 64-bit ints, but from what I recall I treated booleans as atomic and not needing Interlocked or locks, but floats I did. That CPU caching detail level though sounds mighty insidious. I also forged ahead and used TPL, async/await, Rx (Observables are pretty awesome)... But now I live on Erlang's BEAM (via Elixir) and I freaking love it. The real gain for me is that I found I didn't need mutexes, locks, critical sections, etc., because the super lightweight thread-like Erlang processes (not OS processes) themselves run in parallel but each one runs in a single-threaded manner. This effectively turns each process itself into its own critical section, and it's this aspect that I personally have found extremely valuable.

bonzini 10y ago
Do your Erlang processes never execute an asynchronous (await-like) operation? If they did, any shared data would need to be protected by a mutex just as like as with multiple OS threads.
- ibgib 10y ago
  > Do your Erlang processes never execute an asynchronous (await-like) operation? If they did, any shared data would need to be protected by a mutex just as like as with multiple OS threads. I think perhaps what you are getting at is that there is still need for coordination for parallel processes, and that is totally correct. But each BEAM(!) process has its own non-shared memory and runs on a single thread. This is my point in that it doesn't need the mutex because the process' execution is its own critical section. I've structured my ibGib engine to be more functional and the need for the mutex/critical sections has disappeared for me. For example, check out this SO post that I just googled: http://stackoverflow.com/questions/28554114/applying-a-mutex-into-an-erlang-example http://stackoverflow.com/questions/28554114/applying-a-mutex... In the answer, he tells the OP (who thinks he needs a mutex) that what he can actually do is just do the work in the process. This kind of understanding of single-threaded "bounded context"-like execution, combined with ubiquitous immutability has been massively helpful for me with ibGib. In C# for example, I was trying to apply `Task<ib>` all over the place (yes it is non-idiomatic lowercasing of the interface...totally aware of this sin in C#). And so even with async/await awesomeness, the code is just riddled with async/await, blah blah blah. The same goes for using Rx in C# for a "microservices" engine (I started it before that was a fad and I called them autonomous services lol) and my more recent POC with Rxjs in TypeScript/JavaScript. The point is the whole approach of concurrency and whatnot are "addons" in the form of mutexes, semaphores, critical sections, locks, barriers (I've used quite a few over the years). But with programming on the BEAM(!) with Elixir and Erlang, parallel coding is just a more natural experience, because it's just how it's done. In my view, this largely stems from the aspect I mentioned: that they have parallel processes, each running its own memory and on a single thread, communicating with message passing (now called the Actor Model but they didn't know that at the time). On top of this fundamental design decision, they built up an awesome infrastructure with OTP, Supervision trees, and more. But anyway, I didn't mean to write that much - but it's just been a delightful experience, since my very first application in Delphi was a transcription app with a from-scratch multi-threaded realtime document checker (like a spell checker, but more pluggable and geared towards transcription with text expansion which is a totally different use case than existing off-the-shelf spell checkers).

jaegerpicker 10y ago

It's a cool series. Mostly review for me but I certainly passed it to my team to review. Ultimately I say that 99% of the time the correct answer is to not use raw threads and never share memory. There are much better concurrency and parallel patterns out there. Akka(.net in this case), Coroutines and Channels, Erlang Actors, Clojure's core.async, .net's async/await even. Honestly if you are reaching for threads you probably should ask yourself if some other way would be better and safer.

TazeTSchnitzel 10y ago

“Newbie” question: if reading a value without having a lock on it can give you a torn read, wouldn't it be possible to crash the CLR by getting a torn read on a reference? Or, does it account for this somehow?

Eyas 10y ago
A reference fits perfectly in a word-sized chunk. A pointer/reference is typically 32 or 64-bits wide, since it is simply pointing to a location in memory.
dsp1234 10y ago
from the ECMA-335 standard (which is not the latest reference, but is easily accessible): "A conforming CLI shall guarantee that read and write access to properly aligned memory locations no larger than the native word size (the size of type native int) is atomic (see §I.12.6.2) when all the write accesses to a location are the same size." Since references are the native word size, the CLR will ensure that there is no tearing. Note that it is possible to produce a torn read variables that are larger than a native-int, even of native types such as decimal[1]. [0] - http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf http://www.ecma-international.org/publications/files/ECMA-ST... section I.12.6.6, page 102 [1] - http://stackoverflow.com/questions/23262513/reproduce-torn-reads-of-decimal-in-c-sharp http://stackoverflow.com/questions/23262513/reproduce-torn-r...

sundvor 10y ago

Great article. I learned a lot from reading this; being relatively new to C#, now I certainly know what to look for when coming across multithreading.