[swift-users] Data races with copy-on-write

[Jordan Rose]

> On Dec 5, 2017, at 16:24, Romain Jacquinot <rjacquinot at me.com> wrote:
> 
>> Ah, it's a Swift bug in the SynchronizedArray / MyClass cases
> 
> If I remove the intermediate copy in the myArray getter, should it — without Swift bug — also return a unique copy?
> 
> public var myArray: Array<Int> {
>     lock.lock()
>     defer { lock.unlock() }
>     _myArray
> }

Yes. As soon as you treat it as a value (including returning it), it should be independent. The assignment to a local variable doesn't really add anything interesting, and the optimizer will throw it out.

> By the way, here is a simpler sample code where TSan also detects a race condition:
> 
> // Code sample A
> 
> var array: [Int] = [1, 2, 3]
> 
> let iterations = 1_000_000
> 
> var copy1 = array
> q1.async {
>     for i in 0..<iterations {
>         copy1.append(i)
>     }
> }
> 
> var copy2 = array
> q2.async {
>     for i in 0..<iterations {
>         copy2.append(i)
>     }
> }
> 
> var copy3 = array
> q3.async {
>     for i in 0..<iterations {
>         copy3.append(i)
>     }
> }
> 
> for i in 0..<iterations {
>     array.append(i)
> }
> 
> q1.sync {}
> q2.sync {}
> q3.sync {}
> NSLog("done")

Yes, this would be failing for the same reason (didn't test it). The mutations are operating on different Array variables, but they start off sharing the same storage, and that means we can get into the same situation where one queue thinks it can modify the storage in place while another queue is still copying the initial contents.


> I can avoid the race condition to occur by using a capture list:
> 
> // Code sample B
> 
> var array: [Int] = [1, 2, 3]
> 
> let iterations = 1_000_000
> 
> q1.async { [array] in
>     for i in 0..<iterations {
>         var copy = array
>         copy.append(i)
>     }
> }
> 
> q2.async { [array] in
>     for i in 0..<iterations {
>         var copy = array
>         copy.append(i)
>     }
> }
> 
> q3.async { [array] in
>     for i in 0..<iterations {
>         var copy = array
>         copy.append(i)
>     }
> }
> 
> for i in 0..<iterations {
>     array.append(i)
> }
> 
> q1.sync {}
> q2.sync {}
> q3.sync {}
> NSLog("done")
> 
> or by using a constant copy, which, if I’m not wrong, should behave the same way than the capture list:
> 
> // Code sample C
> 
> var array: [Int] = [1, 2, 3]
> 
> let iterations = 1_000_000
> 
> let copy1 = array
> q1.async {
>     var copy1 = copy1
>     for i in 0..<iterations {
>         copy1.append(i)
>     }
> }
> 
> 
> let copy2 = array
> q2.async {
>     var copy2 = copy2
>     for i in 0..<iterations {
>         copy2.append(i)
>     }
> }
> 
> let copy3 = array
> q3.async {
>     var copy3 = copy3
>     for i in 0..<iterations {
>         copy3.append(i)
>     }
> }
> 
> for _ in 0..<iterations {
>     array.append(array.count)
> }
> 
> q1.sync {}
> q2.sync {}
> q3.sync {}
> NSLog("done")

Yep, these two are equivalent, at least as far as this issue goes. This dodges the issue because the capture or the 'let' keeps an extra reference to the original array at all times, meaning that none of the mutations are ever done in-place.

Thank you again for catching this! (It's also possible we've already fixed it on master, but it's still worth having a bug report so we can add a regression test.)

Jordan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-users/attachments/20171205/8e89f37e/attachment.html>