Skip to content

Commit fc5b4fe

Browse files
authored
Merge pull request #3975 from DougGregor/rdar-27541751-error-bridging-race
Eliminate a race condition in NSError bridging
2 parents 5738e1f + ad4e777 commit fc5b4fe

File tree

2 files changed

+13
-12
lines changed

2 files changed

+13
-12
lines changed

stdlib/public/runtime/ErrorObject.mm

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,10 @@ - (NSInteger)code {
7979
- (NSDictionary*)userInfo {
8080
auto error = (const SwiftError*)self;
8181
auto userInfo = error->userInfo.load(SWIFT_MEMORY_ORDER_CONSUME);
82-
83-
if (userInfo) {
84-
// Don't need to .retain.autorelease since it's immutable.
85-
return (NSDictionary*)userInfo;
86-
} else {
87-
// -[NSError userInfo] never returns nil on OS X 10.8 or later.
88-
NSDictionary *emptyDict = SWIFT_LAZY_CONSTANT(@{});
89-
return emptyDict;
90-
}
82+
assert(userInfo
83+
&& "Error box used as NSError before initialization");
84+
// Don't need to .retain.autorelease since it's immutable.
85+
return (NSDictionary*)userInfo;
9186
}
9287

9388
- (id)copyWithZone:(NSZone *)zone {
@@ -319,8 +314,11 @@ typedef SWIFT_CC(swift)
319314
static id _swift_bridgeErrorToNSError_(SwiftError *errorObject) {
320315
auto ns = reinterpret_cast<NSError *>(errorObject);
321316

322-
// If we already have a domain set, then we've already initialized.
323-
if (errorObject->domain.load(SWIFT_MEMORY_ORDER_CONSUME))
317+
// If we already have a domain and userInfo set, then we've already
318+
// initialized.
319+
// FIXME: This might be overly strict; can we look only at the domain?
320+
if (errorObject->domain.load(std::memory_order_acquire) &&
321+
errorObject->userInfo.load(std::memory_order_acquire))
324322
return ns;
325323

326324
// Otherwise, calculate the domain and code (TODO: and user info), and
@@ -334,6 +332,10 @@ static id _swift_bridgeErrorToNSError_(SwiftError *errorObject) {
334332
NSDictionary *userInfo =
335333
swift_stdlib_getErrorUserInfoNSDictionary(value, type, witness);
336334

335+
// Never produce an empty userInfo dictionary.
336+
if (!userInfo)
337+
userInfo = SWIFT_LAZY_CONSTANT(@{});
338+
337339
// The error code shouldn't change, so we can store it blindly, even if
338340
// somebody beat us to it. The store can be relaxed, since we'll do a
339341
// store(release) of the domain last thing to publish the initialized

validation-test/stdlib/ErrorProtocol.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// RUN: %target-run-simple-swift
22
// REQUIRES: executable_test
3-
// REQUIRES: rdar27541751
43
// REQUIRES: objc_interop
54

65
import SwiftPrivate

0 commit comments

Comments
 (0)