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

tcp provider: Early remove ep from polling after shutdown #5708

Merged
merged 1 commit into from
Mar 9, 2020

Conversation

bsbernd
Copy link

@bsbernd bsbernd commented Mar 6, 2020

I noticed that the polling thread in our applicatioon
is spinning for some time after calling fi_shutdown().
Reason is that we only call fi_close() after all references
to our internal connection handling are dropped and that
can be much later than the call of fi_close() and the
FI_SHUTDOWN event handling. Our polling thread then
started to spin without ever going to sleep.
The solution here is to remove the endpoint from polling
after the TCPX_EP_SHUTDOWN flag is set and only after
another round of polling in tcpx_progress() - similar to
what the socket provider does.

Signed-off-by: Bernd Schubert [email protected]

I noticed that the polling thread in our applicatioon
is spinning for some time after calling fi_shutdown().
Reason is that we only call fi_close() after all references
to our internal connection handling are dropped and that
can be much later than the call of fi_close() and the
FI_SHUTDOWN event handling. Our polling thread then
started to spin without ever going to sleep.
The solution here is to remove the endpoint from polling
after the TCPX_EP_SHUTDOWN flag is set and only after
another round of polling in tcpx_progress() - similar to
what the socket provider does.

Change-Id: I66ea20bf344ae8f74a67532657d0e1d199638391
Signed-off-by: Bernd Schubert <[email protected]>
@bsbernd
Copy link
Author

bsbernd commented Mar 6, 2020

This is the updated patch. I'm sorry again for the last patch, I hadn't noticed that master had heavily changed when the merge conflict was auto-resolved. It now also uses the existing ep->cm_state.

@shefty shefty merged commit 533c5c2 into ofiwg:master Mar 9, 2020
@shefty
Copy link
Member

shefty commented Mar 9, 2020

thanks

@bsbernd
Copy link
Author

bsbernd commented Mar 9, 2020

Thank you!

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