[stdlib] Fix a race

Lock-free programming is almost always a bug.
Fixes <rdar://25398370> Data Race in StringBuffer.append (found by TSan)

Author: Dave Abrahams

stdlib/public/core/StringCore.swift

@@ -418,7 +418,10 @@ public struct _StringCore {
   ///   the existing buffer's storage.
   mutating func _claimCapacity(
     _ newSize: Int, minElementWidth: Int) -> (Int, UnsafeMutableRawPointer?) {
-    if _fastPath((nativeBuffer != nil) && elementWidth >= minElementWidth) {
+    if _fastPath(
+      (nativeBuffer != nil) && elementWidth >= minElementWidth
+      && isKnownUniquelyReferenced(&_owner)
+    ) {
       var buffer = nativeBuffer!
 
       // In order to grow the substring in place, this _StringCore should point

test/stdlib/NewStringAppending.swift

@@ -38,7 +38,7 @@ func repr(_ x: NSString) -> String {
 func repr(_ x: _StringCore) -> String {
   if x.hasContiguousStorage {
     if let b = x.nativeBuffer {
-    var offset = x.elementWidth == 2
+    let offset = x.elementWidth == 2
       ? b.start - UnsafeMutableRawPointer(x.startUTF16)
       : b.start - UnsafeMutableRawPointer(x.startASCII)
       return "Contiguous(owner: "
@@ -110,14 +110,8 @@ print("\(repr(s))")
 s += "C"
 print("\(repr(s))")
 
-/// An additional reference to the same buffer doesn't, by itself,
-/// impede the use of available capacity
 var s1 = s
 
-// CHECK-NEXT: String(Contiguous(owner: .native@[[buffer4]][0...50], capacity = 96))
-s += "F"
-print("\(repr(s))")
-
 // CHECK-NEXT: String(Contiguous(owner: .native@[[buffer4]][0...49], capacity = 96))
 print("\(repr(s1))")
 

validation-test/stdlib/StringSlicesConcurrentAppend.swift

@@ -13,17 +13,21 @@ import Glibc
 var StringTestSuite = TestSuite("String")
 
 extension String {
-  var bufferID: UInt {
-    return unsafeBitCast(_core._owner, to: UInt.self)
-  }
   var capacityInBytes: Int {
     return _core.nativeBuffer!.capacity
   }
 }
 
-// Swift.String has an optimization that allows us to append to a shared string
-// buffer.  Make sure that it works correctly when two threads try to append to
-// different non-shared strings that point to the same shared buffer.
+// Swift.String used to hsve an optimization that allowed us to append to a
+// shared string buffer.  However, as lock-free programming invariably does, it
+// introduced a race condition [rdar://25398370 Data Race in StringBuffer.append
+// (found by TSan)].
+//
+// These tests verify that it works correctly when two threads try to append to
+// different non-shared strings that point to the same shared buffer.  They used
+// to verify that the first append could succeed without reallocation even if
+// the string was held by another thread, but that has been removed.  This could
+// still be an effective thread-safety test, though.
 
 enum ThreadID {
   case Primary
@@ -79,11 +83,6 @@ func sliceConcurrentAppendThread(_ tid: ThreadID) {
       secondaryString = privateString
     }
     barrier()
-    if tid == .Primary {
-      expectTrue(
-        (privateString.bufferID == sharedString.bufferID) !=
-          (secondaryString.bufferID == sharedString.bufferID))
-    }
   }
 }