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

Support for Linux's abstract sockets #2526

Closed
nerdrew opened this issue Jan 12, 2021 · 9 comments · Fixed by #2564
Closed

Support for Linux's abstract sockets #2526

nerdrew opened this issue Jan 12, 2021 · 9 comments · Fixed by #2564
Labels

Comments

@nerdrew
Copy link

nerdrew commented Jan 12, 2021

Linux supports listening on an abstract socket that is not represented by a path on the filesystem (https://man7.org/linux/man-pages/man7/unix.7.html). I think ruby's UNIXSocket support abstract sockets, but some of the File.exist? type checks in puma fail because abstract sockets start with a NUL byte "\0". Thoughts on supporting abstract sockets?

@nateberkopec
Copy link
Member

Nothing against including it. What's the use case?

@nerdrew
Copy link
Author

nerdrew commented Jan 13, 2021

I think our use case is using a unix socket with a read-only file system.

@cjlarose
Copy link
Member

@nerdrew What have you tried specifically in order to get puma to try to listen to a unix socket in an abstract namespace? I'm kinda curious what the API would look like for us to implement it.

I think the most straightforward way to allow users to use abstract sockets would probably be something like

# abstract_socket.rb
bind "unix://\0puma.sock"
bundle exec ruby -Ilib bin/puma -C abstract_socket.rb test/rackup/hello.ru

The literal leading NUL byte seems like it'd be accepted by Ruby's UnixSocket according to the discussion in the issue on the Ruby bug tracker.

That fails in puma right now with

Traceback (most recent call last):
        4: from bin/puma:10:in `<main>'
        3: from /Users/chris.larose/dev/puma/lib/puma/cli.rb:80:in `run'
        2: from /Users/chris.larose/dev/puma/lib/puma/launcher.rb:179:in `run'
        1: from /Users/chris.larose/dev/puma/lib/puma/launcher.rb:382:in `set_process_title'
/Users/chris.larose/dev/puma/lib/puma/launcher.rb:382:in `setproctitle': string contains null byte (ArgumentError)

From searching around, it seems like it's a common idiom to accept @ as the leading character to indicate an abstract socket, so on the CLI, maybe we'd accept

puma -b unix://@puma.sock

Does that look like about what you'd expect?

@nerdrew
Copy link
Author

nerdrew commented Jan 13, 2021

Yup, the leading @ looks like what I'd expect. Funny enough, I'm only doing this with a puma server in tests right now, so I'm bypassing most of the CLI stuff and adding the unix listener directly (and yes, I was using @ and then doing the replacement myself, but it would be nicer if that happened in puma I think):

      server = Puma::Server.new ->(env) {
        if env['PATH_INFO'] == '/ready'
          response
        else
          [404, {}, ["Not found.\n"]]
        end
      }
      server.add_unix_listener(socket_path.sub(/^@/, "\0"))
      server.run

Error:

          ArgumentError:
            path name contains null byte
          # ./.bundle_local/ruby/2.7.0/gems/puma-5.1.1/lib/puma/binder.rb:361:in `exist?'
          # ./.bundle_local/ruby/2.7.0/gems/puma-5.1.1/lib/puma/binder.rb:361:in `add_unix_listener'

@cjlarose
Copy link
Member

That makes sense. Are you interested in open a PR for this feature yourself? Otherwise, we can leave the issue up until another contributor has the time to commit to it.

@cjlarose cjlarose changed the title Does puma support Linux's abstract sockets? Support for Linux's abstract sockets Jan 13, 2021
@MSP-Greg
Copy link
Member

I got this working with minimal changes to the code in PR #2519 (using Windows WSL2/Ubuntu), which uses the 'new test framework'. Both hot & phased restarts also worked.

Since it has 'central code' for creating both servers and clients, it's relatively easy (but not trivial) to make the change.

Currently, :ssl, :tcp & :unix are the options for binding, I'll add an option of :aunix for abstract unix sockets...

@nateberkopec
Copy link
Member

Hmmm, does it make sense to add a whole new option? Is an abstract unix socket that different from a filesystem-backed one?

@MSP-Greg
Copy link
Member

Hmmm, does it make sense to add a whole new option?

Sorry, ignore that, I was speaking of CI testing code, not Puma runtime code. Puma runtime code should treat a UnixSocket bind starting with '@' as abstract, that's the only change needed.

But, I'm not sure about abstract UnixSocket binding and systemd, etc.

@nateberkopec
Copy link
Member

I was speaking of CI testing code, not Puma runtime code

That's kinda what I figured but just wanted to make it clear 👍

MSP-Greg added a commit to MSP-Greg/puma that referenced this issue Mar 2, 2021
MSP-Greg added a commit to MSP-Greg/puma that referenced this issue Mar 3, 2021
MSP-Greg added a commit to MSP-Greg/puma that referenced this issue Mar 15, 2021
MSP-Greg added a commit to MSP-Greg/puma that referenced this issue Apr 21, 2021
MSP-Greg added a commit to MSP-Greg/puma that referenced this issue Apr 24, 2021
nateberkopec pushed a commit that referenced this issue Apr 24, 2021
* Support Linux's abstract sockets

Closes #2526

* Add two simple UNIXSocket tests
JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this issue Sep 9, 2022
* Support Linux's abstract sockets

Closes puma#2526

* Add two simple UNIXSocket tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants