-
Notifications
You must be signed in to change notification settings - Fork 3k
Unregister gen_server name in terminate #10285
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
Currently, for every registered process there's a potential race condition where when the process terminates, the supervisor can restart it before the registry processes the DOWN message. This isn't possible to avoid in all situations, but for servers that trap_exit and process `terminate` calls, this is entirely possible. This change implements this adding a call to `gen:unregister_name` just after users `terminate` callback is processed.
CT Test Results 2 files 97 suites 1h 4m 51s ⏱️ Results for commit 83e8109. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts
// Erlang/OTP Github Action Bot |
|
@michalmuskala why only There is one potential problem with doing it this way. Very contrived, though, so I'm just pointing it out.
As I said, this is contrived, but we don't know what exists out there in the wild. AFAICT, there is nothing in the documentation saying that sth like this should not be done. |
|
I agree this should be done for all standard behaviours. I published this as a sort-of proposal, happy to handle the other behaviours, if the OTP team agrees this is a good idea in principle. |
|
@michalmuskala should the registry instead check if the process is alive before returning it? That’s what we do for Elixir’s Registry. |
|
Does it also check that on register commands and then just clears the registration to avoid returning |
|
Yes, all operations that lookup in the registry check if it is alive before returning, although it is a local registry. Unsure about the trade-offs in a distributed one. |
|
I agree this is a better approach, I'm going to close this |
Currently, for every registered process there's a potential race condition where when the process terminates, the supervisor can restart it before the registry processes the DOWN message.
This isn't possible to avoid in all situations, but for servers that trap_exit and process
terminatecalls, this is entirely possible.This change implements this adding a call to
gen:unregister_namejust after usersterminatecallback is processed.