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

Fixes crash when trying to close bipc socket that is yet to be accepted #413

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@baxtersa
Copy link

baxtersa commented May 13, 2015

No description provided.

@gdamore

This comment has been minimized.

Copy link
Contributor

gdamore commented May 13, 2015

Looks pretty good. Do the other transports have a similar problem?

@baxtersa

This comment has been minimized.

Copy link

baxtersa commented May 13, 2015

TCP does, but @vseryakov has an open pull request #410 that addresses that transport. I don't know about ws or tcpmux as I'm tracking the most recent beta release with some cherry-picked commits, not master.

@gdamore

This comment has been minimized.

Copy link
Contributor

gdamore commented May 13, 2015

Ok, so I'll have a go at merging these pull requests in the next day or so, once I get a few spare cycles.

@baxtersa

This comment has been minimized.

Copy link

baxtersa commented May 19, 2015

Before we merge this in, it looks like this and #410 fail on windows due to some os differences in how they set errno's for named pipes. I'm not familiar with Windows system calls at all, but I'm starting to look into it myself. If you're more familiar I'd gladly appreciate the help. I'll comment on the TCP pull request as well.

Sam Baxter and others added some commits May 22, 2015

Merge pull request #1 from baxtersa/bugfix/inproc-crash-fix
Fixes crash on rebinding an inproc socket after original bindee closes.
Merge branch 'master' of https://github.com/nanomsg/nanomsg
Conflicts:
	src/transports/inproc/binproc.c
	src/transports/inproc/cinproc.c
	tests/inproc.c
@gdamore

This comment has been minimized.

Copy link
Contributor

gdamore commented Oct 21, 2015

So, I was looking at this, and I think the inproc changes you have made are unrelated. Please separate those out so that I only have the ipc related change and test, and then I'll merge.

@baxtersa

This comment has been minimized.

Copy link

baxtersa commented Oct 21, 2015

Sorry, looks like those inproc changes are whitespace issues. I'll make the change when I have time to get around to it, don't know how soon that will be though as I'm getting close to a release at work.

@gdamore

This comment has been minimized.

Copy link
Contributor

gdamore commented Oct 21, 2015

Sure no sweat. I could probably make the changes myself if you don’t have
time.

On Wed, Oct 21, 2015 at 12:07 PM, Sam Baxter notifications@github.com
wrote:

Sorry, looks like those inproc changes are whitespace issues. I'll make
the change when I have time to get around to it, don't know how soon that
will be though as I'm getting close to a release at work.


Reply to this email directly or view it on GitHub
#413 (comment).

@gdamore

This comment has been minimized.

Copy link
Contributor

gdamore commented Nov 9, 2015

This got fixed in

garrett@Triton{4}% git log src/transports/ipc/bipc.c
commit 827d434
Author: Garrett D'Amore garrett@damore.org
Date: Thu Oct 22 16:04:07 2015 -0700

fixes #469 assertion failure in ipc_shutdown

@gdamore gdamore closed this Nov 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment