Thank you Jordan. I’ve filed a bug at bugs.swift.org <http://bugs.swift.org/>: https://bugs.swift.org/browse/SR-6543 <https://bugs.swift.org/browse/SR-6543>.
> On Dec 6, 2017, at 2:47 AM, Jordan Rose <jordan_r...@apple.com> wrote: > > > >> On Dec 5, 2017, at 16:24, Romain Jacquinot <rjacqui...@me.com >> <mailto: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
