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

Allow binding UNIX domain sockets #60

Closed
wants to merge 1 commit into from

Conversation

spladug
Copy link

@spladug spladug commented Apr 8, 2016

As mentioned in #58, this adds support for bound sockets being UNIX domain sockets. This is something that I needed for an application on our end and think might be generally useful. This works by extending the regex in argument parsing to accept filenames that don't have colons in them as UNIX domain sockets. The state.bind stuff is then modified to be inet or unix bind objects which abstracts out the logic of how to create/bind each type of socket properly. If binding the socket results in EADDRINUSE, we'll do the connect, stat, unlink dance to try and clean up old dead sockets. The address family is added to the child environment for each socket so that workers can know which family to bind with more easily.

Issues/questions:

  • I can't get the tests to run locally without errors with or without this patch, so I'm not clear on their status. Will check with Travis once that runs. It looks like the tests are failing to install packages etc. on various versions, but where they install everything is checking out.
  • There's some duplication between the command socket binding/cleanup code and the unix socket cleanup code in this patch. I wasn't clear how I could de-duplicate this and maintain the descriptive error messages.
  • I'm not super fond of the bind.rb file or how that stuff is structured, if you've any alternatives I'm all ears.
  • I've tested in place upgrades but there might be more cases I'm not thinking about there.

@spladug spladug force-pushed the unix-sockets branch 5 times, most recently from a88acbd to 8f38ac5 Compare April 8, 2016 16:39
Some applications need to be mediated by another process/processes on
the same machine.
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

3 participants