Don’t mix await and compound assignment

re: Don’t mix await and compound assignment

JaredPar MSFT — Mon, 18 Feb 2013 16:16:57 GMT

@Leonid it is indeed atomic but that's not the code that is executed here. If you look at the IL it's essentially

ldfield m_sum

await y

add

stfield m_sum

The "add" IL instruction operates on the 2 values which are pushed on the IL stack. The m_sum value is pushed atomically but it is also cached on the stack. While waiting for `await y` to finish, it's very possible that another operation will start running which modifies m_sum. This will modify the field only and not the value which is cached on the stack. Hence when it resumes it will add the previous value of m_sum to 'await y' and erase the intermediate m_sum value

re: Don’t mix await and compound assignment

Leonid Tcarev — Mon, 18 Feb 2013 10:34:45 GMT

@Jared, why?

add m_sum, eax is atomic.

re: Don’t mix await and compound assignment

JaredPar MSFT — Fri, 15 Feb 2013 16:51:14 GMT

@Leonid the write of the int32 value is atomic but the read of the original value can occur on a separate turn from the write. This is what makes it unsafe

re: Don’t mix await and compound assignment

Leonid Tcarev — Fri, 15 Feb 2013 10:56:43 GMT

>The first add could get preempted after it reads m_sum, and the second thread could run and update >m_sum, and then the first thread would trample the second thread's update.

Adding of something to int32 bit variable (not property) should be atomic at x86

re: Don’t mix await and compound assignment

Ian Obermiller — Wed, 13 Feb 2013 18:48:42 GMT

Oh, right. That is what makes it even more interesting (and deadly) I suppose. Thanks!

re: Don’t mix await and compound assignment

JaredPar MSFT — Tue, 12 Feb 2013 19:25:47 GMT

@Ian

This is actually a bug which happens equally as well for single threaded and multi-threaded code. The repro code I showed all executes on a single thread.

re: Don’t mix await and compound assignment

Ian Obermiller — Tue, 12 Feb 2013 19:23:25 GMT

Since you are referencing multi-threading bugs, isn't the example with a temporary value still unsafe? The first add could get preempted after it reads m_sum, and the second thread could run and update m_sum, and then the first thread would trample the second thread's update. So the correct code would need some kind of synchronization, like Interlocked.Add.

The main point though is a good one, and problems like this are becoming much easier with await.