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

Registrar.deregister ownership transfer #22

Closed
Luminarys opened this issue May 27, 2017 · 6 comments
Closed

Registrar.deregister ownership transfer #22

Luminarys opened this issue May 27, 2017 · 6 comments

Comments

@Luminarys
Copy link
Contributor

The reasoning of "Sockets should only be deregistered when the caller is done with them." makes sense, however this prevents use cases where sockets are transferred between polls. Is this limitation intentional?

@andrewjstone
Copy link
Owner

Hi @Luminarys,

I'm not sure I follow you here. You can transfer sockets between polls. A socket stays registered until it's deregistered. You use reregister to reuse the socket for this purpose. Note also, that you can call register, reregister, or deregister from any thread where you have a clone of the registrar.

@Luminarys
Copy link
Contributor Author

The issue (I think?) would be that if I have a poll on thread A, which has some socket registered to it, and I want to transfer this socket to some thread B which has its own poll. If I simply transferred the socket to B and reregistered it without deregistering it on A, would that not cause events for the socket to be reported to the polls on both thread A and B?

@andrewjstone
Copy link
Owner

Oh, I see. I never intended sockets to be moved around pollers like that. My use case is that there is a single poller, and events can be routed to different threads where the sockets live via channels. Polling eats threads since it's the only thing they can be used for, and decoding messages and doing work on that thread delays any future polling. However, that's just how I use Amy, and in no way is necessarily "the right way".

I agree that your use case is valid. I don't think you'd want the same socket registered with multiple poll threads. At least with epoll, those events would be reported to both pollers. All that being said, I don't see any real reason why the API can't be changed to just take a reference to the socket in deregister. I think that would fix things so that your use case works, and I don't believe it should cause any other problems. I just looked through git blame to see if there was a specific reason I did it that way and couldn't find any safety issues. It's probably just historical at this point.

@Luminarys
Copy link
Contributor Author

Unless you're already working on it then, I'll submit a PR with the appropriate changes if that's alright.

@andrewjstone
Copy link
Owner

That would be great. Much appreciated.

@andrewjstone
Copy link
Owner

Fixed by #23

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

No branches or pull requests

2 participants