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.

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

NSNotificationCenter should automatically unregister released observers #481

Closed
triplef opened this issue Dec 19, 2024 · 9 comments
Closed
Assignees

Comments

@triplef
Copy link
Member

triplef commented Dec 19, 2024

On newer Apple OS releases, NSNotificationCenter no longer requires users to unregister observers that get released. It would be helpful if GNUstep could do the same.

If your app targets iOS 9.0 and later or macOS 10.11 and later, you do not need to unregister an observer that you created with this function. If you forget or are unable to remove an observer, the system cleans up the next time it would have posted to it.

https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415360-addobserver?language=objc

@rfm
Copy link
Contributor

rfm commented Dec 19, 2024

The obvious way to implement this sort of thing would be rewriting to use weak references to observers, but that's a runtime dependent feature.
As the gnustep project has a principle of maintaining backward compatibility as much as possible, this looks like something we shouldn't do ... unless you have an efficient implementation for gcc/gnu runtime.

@triplef
Copy link
Member Author

triplef commented Dec 19, 2024

Thanks for your feedback @rfm. I’m not familiar with what weak reference support looks like with GCC, but since this is a new-ish feature on Apple platforms as well, wouldn’t it be ok to support it with libobjc2 only if it’s only feasible there? I see this as an improved implementation, similar to the new KVO implementation that also requires runtime features. Normal add/remove observer would of course still work with all runtimes.

For us the main reason we’d like to see this in GNUstep is that our devs mainly write code on Apple platforms, and if they forget to remove an observer somewhere we won’t notice on those platforms, plus it’s often difficult to catch these mistakes as the app might not crash immediately if the observer is released. Having this work the same across (our) platforms would eliminate this potential source of errors.

@rfm
Copy link
Contributor

rfm commented Dec 20, 2024

Generally it's a big problem for maintainability and support to have different behaviors on different systems and is antithetical to one of the guiding principles of the project: portability ... we should be able to write GNUstep code and it should run on all platforms where GNUstep core libraries run.

We already have far too much stuff that is not completely portable (mostly due to external dependencies), and the worse it gets the greater the pushback against anything more, so the fact that we are already trying to integrate a new KVO implementation is a strong reason not to add more runtime specific code. Arguably we should iron out quite a lot of the outstanding differences before doing anything that adds new ones.

That being said, I think this could be done in two stages:

  1. add libobjc2 compatible weak pointer support to the base library when built for gcc and the gnu runtime (objc_loadWeak(), objc_storeWeak(), and objc_delete_weak_refs() should be sufficient).
  2. use that weak pointer support to update NSNotificationCenter so that it removes observers once they have been deallocated.

@rfm
Copy link
Contributor

rfm commented Dec 20, 2024

I'd be very happy to see a contribution of (1) above, since that would also allow us to fix weak pointer based incompatibilities elsewhere in base, I think a C language implementation based on the algorithms in arc.mm in libobjc2 would be great for portability.

@rfm
Copy link
Contributor

rfm commented Dec 23, 2024

Actually, I think I'll look into adding weak reference support (emulation of the runtime features) for use with gcc and the gnu runtime anyway, because having working weak pointer versions of NSPointerArray, NSHashTable and NSMapTable would be good.

@triplef
Copy link
Member Author

triplef commented Dec 23, 2024

Great, thank you.

@rfm
Copy link
Contributor

rfm commented Dec 24, 2024

I have preliminary (working) code for weak references with gcc in a new 'weakref' branch. As far as I know the only problem with it is that it's currently going to be too slow because every object deallocation involves locking and a hash table lookup. In libobjc2 this is mostly avoided by marking each weakly referenced object, and using a lock-free check at deallocation to see whether the object being deallocated has weak references. Probably we need something similar.

@rfm
Copy link
Contributor

rfm commented Dec 27, 2024

This should now be working on the 'weakref' branch for both the classic gnu runtime and for the modern ruuntime.
There are even some new testcases :-)

@triplef
Copy link
Member Author

triplef commented Dec 29, 2024

Thank you Richard, that’s great!

@rfm rfm closed this as completed Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants