celluloid / reel

UNMAINTAINED: See celluloid/celluloid#779 - Celluloid::IO-powered web server
https://celluloid.io
MIT License
596 stars 87 forks source link

Server::UNIX code to bring in after UNIX sockets work fully under jRuby. #123

Closed digitalextremist closed 9 years ago

digitalextremist commented 10 years ago

This code is ready to follow-up to #121 once celluloid/nio4r#37 is resolved.

Asmod4n commented 10 years ago

One would also have to manually delete the socket file, like

def shutdown
  super
  File.delete(@socket_path) if File.exists? (@socket_path)
end

Dunno if it would be advisable to also do that during startup.

justindarby commented 10 years ago

I just duplicated the efforts of digitalextremist without realizing it to extend Reel::Server into a UNIXServer, @Asmod4n my thought is that you pretty much have to have something in front checking to see if the old process is running, then checking for and unlinking the file if not.. because you can't guarantee shutdown happened. It'd be better just to make people aware they have to clean up after themselves, probably, because there's no built in method to insure the right thing is going to happen for everyone.. and what happens with digitalextremist's / my code is just that... an error on startup that the socket is in use and a reminder to write better code to handle startup conditions.

digitalextremist commented 10 years ago

Hey @justindarby. @stouset wrote the UNIX server code, if I remember correctly. I just ported it to work in a post #121 world. We're still having problems with UNIX socket servers under jRuby though, so this piece was shelved to let #121 pass through and be released without UNIX socket server issues.

Please check into the changes to Server at #121 and #130 by the way.

digitalextremist commented 10 years ago

@Asmod4n I will add that method, or @stouset can, or you can, and by all means submit a PR to this branch directly ( unix_server ) and I will pull that in, and it'll get factored into this PR.

justindarby commented 10 years ago

Yeah, I'm using those changes, although I just brought Reel::Server into another class instead of Real::Server::UNIX. I withdraw my concern about shutdown, the following test case works as expected (shutdown doesn't fire) and I'm new to this code (and have already learned to not trust instincts developed by using conventional ruby):

require 'celluloid/autostart'
require 'celluloid/io'
class BreakingStuff
        include Celluloid
        include Celluloid::Logger
        def initialize
                socket = Celluloid::IO::UNIXServer.new(__FILE__)
        end
        def shutdown
                info "If this prints something, this is why shutdown in Reel::Server::UNIX should not unlink sockets because that file might not be the socket we created."
        end
end
BreakingStuff.supervise_as :broken
Asmod4n commented 10 years ago

@justindarby you forgot

finalizer :shutdown

in there

justindarby commented 10 years ago

Ah, yeah. Okay this is bad then. Throw finalizer :shutdown into the above code and we just unlinked a socket we don't own.

digitalextremist commented 10 years ago

Unrelated: @justindarby out of curiosity, are you using Rubinius, MRI, or somehow using jRuby?

justindarby commented 10 years ago

@digitalextremist Rubinius

justindarby commented 10 years ago

Continually unrelated, @digitalextremist I'm writing an app that either needs nginx sitting in front of it to handle baseline security policies or I have to rewrite equivalent functionality. So, Reel::Server::UNIX = good = keeps my app off the network.

seven1m commented 9 years ago

This looks great.... any idea when it might get merged?

digitalextremist commented 9 years ago

We're waiting for this to be usable under jRuby

On March 4, 2015 10:50:40 AM PST, Tim Morgan notifications@github.com wrote:

This looks great.... any idea when it might get merged?


Reply to this email directly or view it on GitHub: https://github.com/celluloid/reel/pull/123#issuecomment-77219989

digitalextremist commented 9 years ago

Merged into 0.6.0-milestone about to hit master. NOT supporting jRuby. See #179 notes.