Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

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

Closed
wants to merge 26 commits into from

Conversation

digitalextremist
Copy link
Member

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

//de and others added 2 commits December 11, 2013 04:45
This will change as of #121 and be updated in #123 after that.
@Asmod4n
Copy link
Contributor

Asmod4n commented Jan 28, 2014

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
Copy link

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
Copy link
Member Author

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
Copy link
Member Author

@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
Copy link

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
Copy link
Contributor

Asmod4n commented Feb 4, 2014

@justindarby you forgot

finalizer :shutdown

in there

@justindarby
Copy link

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
Copy link
Member Author

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

@justindarby
Copy link

@digitalextremist Rubinius

@justindarby
Copy link

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.

bruz pushed a commit to CXInc/reel that referenced this pull request Dec 10, 2014
Based on celluloid#123, which hasn't been
merged yet.
@seven1m
Copy link

seven1m commented Mar 4, 2015

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

@digitalextremist
Copy link
Member Author

We're waiting for this to be usable under jRuby

On March 4, 2015 10:50:40 AM PST, Tim Morgan [email protected] wrote:

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


Reply to this email directly or view it on GitHub:
#123 (comment)

@digitalextremist digitalextremist added this to the 0.6.5 milestone Mar 22, 2015
@digitalextremist
Copy link
Member Author

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

@digitalextremist digitalextremist mentioned this pull request Mar 24, 2015
6 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants