Skip to content
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.

Sign up for GitHub

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

Jump to bottom

NSNotificationCenter is not thread safe. #2391

Closed
rupertdaniel opened this issue Mar 31, 2017 · 1 comment
Closed

NSNotificationCenter is not thread safe. #2391

rupertdaniel opened this issue Mar 31, 2017 · 1 comment
Assignees
@DHowett-MSFT
Labels
Area-Foundation Task
Milestone
1704

Comments

@rupertdaniel
Copy link

NSNotificationCenter is not thread safe, removing, adding and notifying observers from different threads results in a crash.

On the reference platform NSNotificationCenter is thread safe.
@rajsesh rajsesh assigned DHowett-MSFT and unassigned rajsesh Apr 3, 2017
@rajsesh rajsesh added the Task label Apr 3, 2017
@DHowett-MSFT
Copy link

@rajsesh I was hoping I'd get this one 😉

@rajsesh rajsesh added this to the 1704 milestone Apr 4, 2017
DHowett-MSFT pushed a commit to DHowett-MSFT/WinObjC that referenced this issue Apr 5, 2017
@DHowett
Reimplement NSNotificationCenter
aafd80e 
Fixes microsoft#2391.
DHowett-MSFT pushed a commit to DHowett-MSFT/WinObjC that referenced this issue Apr 5, 2017
@DHowett
Reimplement NSNotificationCenter
d376b84 
Fixes microsoft#2391.
@DHowett-MSFT DHowett-MSFT mentioned this issue Apr 5, 2017
Merged
DHowett-MSFT pushed a commit that referenced this issue Apr 6, 2017
@DHowett
Reimplement NSNotificationCenter (and NSNotification) (#2421)
a2d4b3f 
This commit guts and reimplements both NSNotificationCenter and NSNotification.

NSNotification is supposed to be a class cluster. It it was empirically observed that non-concrete instances could still encode/decode, but would fail with abstract invocation exceptions on the property getter.

NSNotificationCenter is vastly improved. It:
* Uses sets for quick lookup, since notification order bears no guarantees
* Maintains a set of all receivers (for quick name-free removal)
* Maintains a mapping from notification name to interested receivers (for quick dispatch)
* Now supports enqueuing notification blocks on queues
* Fulfills the contractual obligations of `addObserverForName:object:queue:usingBlock:`
* Has no caveats
* Is thread-safe
* Is tested

Fixes #2391.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@DHowett-MSFT DHowett-MSFT

Labels
Area-Foundation Task
Projects
None yet
Milestone
1704
Development

No branches or pull requests
4 participants
@rupertdaniel @ashvarma @DHowett-MSFT @rajsesh