Skip to content

Guard against racy access to NSError.setUserInfoValueProvider. #4558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

jrose-apple
Copy link
Contributor

  • Explanation: Our Error/NSError bridging uses a specific API to provide NSError's userInfo dictionary without having to actually construct a dictionary in Swift. However, this API can only be called once for a particular domain, so we have to make sure our check for an existing provider is synchronized with the setting. Otherwise, we get a race condition that could lead to an over-release.
  • Scope: The bug only occurs when the same Swift error type is thrown on two different threads, and no error of that type has been thrown before, and the timing is exactly wrong. The fix affects getting the user info for any Swift error.
  • Issue: rdar://problem/27541751
  • Reviewed by: @parkera, @DougGregor
  • Risk: Very low.
  • Testing: Re-enabled the regression test that originally caught this issue. Tests passed locally with several runs under GuardMalloc where they had previously failed every fifth run or so.

…lang#4280)

This API is documented in its headers to only allow being called once
for a particular domain, so we have to make sure our check for an
existing provider is synchronized with the setting.

rdar://problem/27541751
(cherry picked from commit af1a815)
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test OS X platform

@jrose-apple jrose-apple merged commit cb109dd into swiftlang:swift-3.0-branch Aug 30, 2016
@jrose-apple jrose-apple deleted the swift-3-racy-NSError-userInfo branch August 30, 2016 21:56
@gparker42
Copy link
Contributor

Several tests are crashing in error-related code after this change. This might only affect un-optimized builds.
https://ci.swift.org/view/swift-3.0-branch/job/oss-swift-3.0_tools-RA_stdlib-DA_test-simulator/51/

@jrose-apple
Copy link
Contributor Author

Aargh. That doesn't make any sense. Thanks, Greg.

@jrose-apple
Copy link
Contributor Author

sigh It's a known issue, rdar://problem/27226313. @slavapestov, were you planning to do anything here?

@jrose-apple
Copy link
Contributor Author

It does only affect builds with unoptimized overlays, by the way.

@slavapestov
Copy link
Contributor

Beyond cherry-picking for the swift 3.0 branch, I wasn't planning on working on SILGen support for factory initializers soon. But if we keep hitting this again and again it might be worth addressing properly.

@jrose-apple
Copy link
Contributor Author

What was the cherry-pick? Disabling tests, or actually fixing the issue? Or at least papering over it?

(@zisko is also concerned about leaving a bot red, so if that cherry-pick isn't going to fix it I'll disable these tests on unoptimized stdlib configurations.)

kateinoigakukun added a commit that referenced this pull request Aug 31, 2022
[pull] swiftwasm from main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants