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

workaround for intermittent failures of tcp_shutdown test on Windows #303

Closed
wants to merge 1 commit into from

Conversation

TTimo
Copy link
Contributor

@TTimo TTimo commented Aug 19, 2014

This gross hack avoids the tcp_shutdown test failure on Windows. It also fixes an abort during shutdown that we encountered in our internal product while using the ipc transport (our internal bug had lower repro, but wasn't as pathological as the tcp_shutdown test - still, it was the same issue).

I'm not suggesting that this be merged, but I'm hoping someone can provide some insight as to what is going on (e.g. what is a non NN_SOCK_SRC_EP doing here) and point towards a better fix.

@TTimo TTimo mentioned this pull request Aug 19, 2014
@djc
Copy link
Member

djc commented Aug 19, 2014

Perhaps you should mention this on the mailing list, as well.

@JackDunaway
Copy link
Contributor

@TTimo - I've likewise been working on this issue, and may have found a fix (linked below) -- perhaps you can give a try? Thanks!

https://github.com/wirebirdlabs/ftw-nanomsg/commit/bb5d37c70800aa620391c80a30b07dcc83501e26

@TTimo
Copy link
Contributor Author

TTimo commented Oct 22, 2014

@wirebirdlabs the commit you linked doesn't apply cleanly against nanomsg master. It looks like some of your changes are already in master, whereas the change is otherwise built on older code. I was able to fix the merge conflicts correctly I believe, but tcp_shutdown still has the same failure pattern that I am working around in this pull request, in nn_sock_shutdown on

nn_assert (src == NN_SOCK_SRC_EP && type == NN_EP_STOPPED);

@wirebirdlabs your merged change against nanomsg master is at https://github.com/TTimo/nanomsg/tree/pull_303_internal_216_wirebirdlabs if you are interested in refining it.

@sustrik your input would be very welcome here. I am working to submit a few more pull requests, but they in part depend on this fix.

@JackDunaway
Copy link
Contributor

@TTimo Great! I'll look into this. Right now, I'm neck-deep in fsm's and usock's getting full Autobahn Test Suite compliance on a new ws:// transport for nanomsg.

@djc
Copy link
Member

djc commented Oct 24, 2014

@wirebirdlabs your ws:// transport sounds exciting, looking forward to it!

@TTimo TTimo closed this Dec 5, 2014
@TTimo TTimo deleted the tcp_shutdown_internal_216_2 branch December 5, 2014 16:54
@TTimo TTimo restored the tcp_shutdown_internal_216_2 branch December 5, 2014 16:57
@TTimo TTimo reopened this Jan 13, 2015
@TTimo
Copy link
Contributor Author

TTimo commented Jan 13, 2015

I may have closed this one by mistake. It's still an issue / fix which we have in our prod code.

@JackDunaway
Copy link
Contributor

This PR might effectively be handled by 7555f49 -- @TTimo , what do you think?

@gdamore
Copy link
Contributor

gdamore commented Oct 25, 2015

Yep, I've resolved this a different way altogether. Closing this one.

@gdamore gdamore closed this Oct 25, 2015
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.

None yet

4 participants