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

Send nonblocking signals to shutdown netceptor instances #26

Closed
wants to merge 1 commit into from

Conversation

squidboylan
Copy link
Contributor

No description provided.

@squidboylan
Copy link
Contributor Author

hmm, this may actually not fix shutdown, but might just prevent the hanging

@ghjm
Copy link
Contributor

ghjm commented Jun 26, 2020

Yeah, I'd like to figure out who's not listening for this and why. But I agree with the idea of protecting the main thread. Maybe it would be better to do this by making the shutdown channels buffered with size 1? That will still avoid blocking on the main thread, but if the goroutine eventually finishes what it's doing and listens to the done channel, it will still get the shutdown request.

@squidboylan
Copy link
Contributor Author

Yeah, I'd like to figure out who's not listening for this and why. But I agree with the idea of protecting the main thread. Maybe it would be better to do this by making the shutdown channels buffered with size 1? That will still avoid blocking on the main thread, but if the goroutine eventually finishes what it's doing and listens to the done channel, it will still get the shutdown request.

Ok i think i figured out what's going on, the signal in runProtocol shuts down that tcp session, but doesnt close the tcp port because we dont have a way to tell it to in https://github.com/project-receptor/receptor/blob/devel/pkg/backends/tcp.go#L84 so i think we need a way to send a signal to close the tcp port rather than just closing a session, however im not really sure how to architect that

@ghjm
Copy link
Contributor

ghjm commented Jun 26, 2020

Ok, can you open an issue with the details and I'll take a look at it?

Also I think the shutdown channels are not being properly initialized.

@squidboylan
Copy link
Contributor Author

Closing this as this PR is likely a deadend, #28 will track future work to fix this

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.

2 participants