Skip to content

Commit 0ec27bd

Browse files
committed
Runtime: Bridging an Error to NSError requires only checking its domain.
The lazy population of the NSError fields is ordered such that the domain is written into the object last with acq/rel ordering, so another thread only needs to check the domain to see whether the initialization has already happened. (The initialization itself is idempotent, so we can optimistically perform the initialization and discard the results if we race.) Checking the user info is redundant, and is also wrong for real NSError objects, since [NSError errorWithDomain:d code:c userInfo:nil] will in fact plant nil in the userInfo field of the object, leading us to attempt to bridge an already-native NSError.
1 parent e02de50 commit 0ec27bd

File tree

1 file changed

+16
-8
lines changed

1 file changed

+16
-8
lines changed

stdlib/public/runtime/ErrorObject.mm

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -424,14 +424,22 @@ typedef SWIFT_CC(swift)
424424
static id _swift_bridgeErrorToNSError_(SwiftError *errorObject) {
425425
auto ns = reinterpret_cast<NSError *>(errorObject);
426426

427-
// If we already have a domain and userInfo set, then we've already
428-
// initialized.
429-
// FIXME: This might be overly strict; can we look only at the domain?
430-
if (errorObject->domain.load(std::memory_order_acquire) &&
431-
errorObject->userInfo.load(std::memory_order_acquire))
427+
// If we already have a domain set, then we've already initialized.
428+
// If this is a real NSError, then Cocoa and Core Foundation's initializers
429+
// guarantee that the domain is never nil, so if this test fails, we can
430+
// assume we're working with a bridged error. (Note that Cocoa and CF
431+
// **will** allow the userInfo storage to be initialized to nil.)
432+
//
433+
// If this is a bridged error, then the domain, code, and user info are
434+
// lazily computed, and the domain will be nil if they haven't been computed
435+
// yet. The initialization is ordered in such a way that all other lazy
436+
// initialization of the object happens-before the domain initialization so
437+
// that the domain can be used alone as a flag for the initialization of the
438+
// object.
439+
if (errorObject->domain.load(std::memory_order_acquire))
432440
return ns;
433441

434-
// Otherwise, calculate the domain and code (TODO: and user info), and
442+
// Otherwise, calculate the domain, code, and user info, and
435443
// initialize the NSError.
436444
auto value = SwiftError::getIndirectValue(&errorObject);
437445
auto type = errorObject->getType();
@@ -463,8 +471,8 @@ static id _swift_bridgeErrorToNSError_(SwiftError *errorObject) {
463471
// We also need to cmpxchg in the domain; if somebody beat us to it,
464472
// we need to release.
465473
//
466-
// Storing the domain must be the LAST THING we do, since it's
467-
// the signal that the NSError has been initialized.
474+
// Storing the domain must be the **LAST THING** we do, since it's
475+
// also the flag that the NSError has been initialized.
468476
CFStringRef expectedDomain = nullptr;
469477
if (!errorObject->domain.compare_exchange_strong(expectedDomain,
470478
(CFStringRef)domain,

0 commit comments

Comments
 (0)