Skip to content

net: Listener sometimes accepts connections after Close #13762

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.

Sign up for GitHub

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

Jump to bottom
Closed
prashantv opened this issue Dec 29, 2015 · 5 comments
Closed

net: Listener sometimes accepts connections after Close #13762

prashantv opened this issue Dec 29, 2015 · 5 comments
Labels
FrozenDueToAge
Milestone
Unplanned

Comments

@prashantv
Copy link
Contributor

I've written a repro test here:
https://github.com/prashantv/listener-repro/blob/master/main.go

It basically does the following:

  • Start a listener
  • Create connectionsBeforeClose connections to the listener (which should succeed)
  • Close the listener
  • Create another connection to the listener

On the listener side, I expect to receive at most connectionsBeforeClose successful connections. Any extra connections causes the runTest method to return an error.

This works as expected on OSX, but on Linux, it seems to receive a successful connection after the listener has been closed.

Some things I noticed:

  • If connectionsBeforeClose is set to 0, then no unexpected connections are made after the listener is closed
  • The issue also disappears if I enableSleep

I would expect that a Listener will not accept any further socket attempts after Close has returned.
prashantv added a commit to uber/tchannel-go that referenced this issue Dec 29, 2015
@prashantv
Avoid race condition between Close and Accept
871a787 
When Close is called on the listener, the goroutine blocked on Accept
does not immediately error out. It's possible that new incoming
connections will be accepted (at least on Linux). See:
golang/go#13762

We can avoid this by making sure that the server is really closed (e.g. not
accepting new connections) in close_test before trying to make a call.
@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Dec 29, 2015
@mikioh mikioh modified the milestones: Unplanned, Go1.7 Dec 29, 2015
@mikioh
Copy link
Contributor

mikioh commented Dec 29, 2015

This is a platform-indepedent issue and a duplicate of #10527.

@mikioh mikioh closed this as completed Dec 29, 2015
@prashantv
Copy link
Contributor Author

@mikioh I saw #10527, but I think this is a little different. #10527 is basically that the socket isn't closed since Accept has a reference to the underlying socket. However, the Accept should return an error when it is scheduled since the socket is closed.

In this case, Accept is not returning an error after the Close call. If you run the repro code, you'll see that it only fails on Linux and not OSX, so I'm not sure if this is a platform independent issue.

@mikioh
Copy link
Contributor

mikioh commented Dec 29, 2015

@prashantv,

Thanks for the notice, hm, perhaps we need to dig accept4 more on both Linux and FreeBSD.

@mikioh mikioh reopened this Dec 29, 2015
@mikioh mikioh changed the title net: Listener sometimes accepts connections after Close on Linux net: Listener sometimes accepts connections after Close Dec 29, 2015
@mikioh mikioh modified the milestones: Go1.7, Unplanned Dec 29, 2015
@mikioh
Copy link
Contributor

mikioh commented Dec 29, 2015

@prashantv,

To conclude, this is a duplicate of #10527. In general this may happen when the shared fdMutex in TCPListener is taken by acceptor goroutine prior to TCPListener.Close like the following:

  • goroutine1: calls netFD.Close. Close sets the state to closed and increments and decrements the ref count, but decref doesn't call netFD.destroy because goroutine2 has a readlock.

See #10527 for further information.

@mikioh mikioh closed this as completed Dec 29, 2015
@mikioh mikioh modified the milestones: Unplanned, Go1.7 Dec 29, 2015
@prashantv prashantv mentioned this issue Dec 29, 2015
Closed
@prashantv
Copy link
Contributor Author

@mikioh Ah, I thought that perhaps Accept would check whether the socket was marked as Closed but that doesn't affect epoll from accepting new connections.

It looks like the other issue has not had any progress since it was last marked as possibly for go 1.5. Can #10527 be prioritized for go 1.7? Thanks

@golang golang locked and limited conversation to collaborators Dec 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@mikioh @prashantv @ianlancetaylor @gopherbot