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

manager: remove hack for unix-socket listener #1541

Closed
wants to merge 1 commit into from

Conversation

LK4D4
Copy link
Contributor

@LK4D4 LK4D4 commented Sep 15, 2016

It was fixed in grpc

@codecov-io
Copy link

codecov-io commented Sep 15, 2016

Current coverage is 53.73% (diff: 100%)

Merging #1541 into master will decrease coverage by 0.08%

@@             master      #1541   diff @@
==========================================
  Files            82         82          
  Lines         13430      13432     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           7228       7218    -10   
- Misses         5219       5227     +8   
- Partials        983        987     +4   

Sunburst

Powered by Codecov. Last update 65af901...e9d3de0

@@ -549,7 +536,7 @@ func (m *Manager) serveListener(ctx context.Context, errServe chan error, proto
// we need to disallow double closes because UnixListener.Close
// can delete unix-socket file of newer listener. grpc calls
// Close twice indeed: in Serve and in Stop.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed, thanks!

It was fixed in grpc

Signed-off-by: Alexander Morozov <lk4d4math@gmail.com>
@stevvooe
Copy link
Contributor

Good byte, dirty hacks.

@aaronlehmann
Copy link
Collaborator

I am not sure if this idea will be popular, but could we instead wrap the listener with something that panics if Close is called twice?

I'm worried that this might regress in grpc in the future, and it will be hard to notice.

Or we could keep the closeOnceListener wrapper to be on the safe side. It's only a few lines of code.

@LK4D4
Copy link
Contributor Author

LK4D4 commented Sep 15, 2016

@aaronlehmann I've opened golang/go#17131
And I think you're right, there is no harm in that wrapper now.

@LK4D4 LK4D4 closed this Sep 15, 2016
@LK4D4 LK4D4 deleted the remove_hack branch September 15, 2016 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants