> On Dec 5, 2017, at 16:24, Romain Jacquinot <rjacqui...@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_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users
