Server#stop! stopping EventMachine reactor

[tbuehlmann]

I came across a situation in which I had an EM reactor running, having multiple apps in it. Under these apps there was a Thin app which I needed to stop at a certain point. Doing so, the whole EM reactor stopped working. Some investigation brought me to Server#stop! and Backends::Base#stop!:

thin/lib/thin/backends/base.rb

Line 91 in 5e56023

EventMachine.stop if EventMachine.reactor_running?

So, is this really intended to stop the whole EM reactor when a single Backend is told to stop?

[ghost]

Interesting. I have a similar situation in which I'm running a several Thin::Server in the same process (and thus, reactor loop), then stoping one of the server objects seems to shut down the EM reactor loop (causing other servers to stop).

One way to fix this would be to have the server (backend, really) remember whether it "owns" the reactor (see here). If it is the one that started the reactor (called EM::run), then it can claim ownership of the reactor, elseit simply adds itself to the reactor. In the case where it doesn't own/start the reactor, we can skip over the reactor shutdown.

So in the case where a single process (and reactor loop) is servicing multiple servers, it is up to the caller to start (and thus "ownn") the EM reactor loop, and none of the server objects will do this.

This should not affect the existing standalone thin application in any way, and only benefits those who (like me) are embedding Thin::Server into their application which runs atop EM.

@macournoyer , thoughts?

[tbuehlmann]

I probably wouldn't go this way. What if you have several EM based libraries running and a Thin server only was the first one being started? Backends::Base#stop is the right thing to do in some cases and one should use it. If one really wants to hard stop a server, let's do so but don't stop the EM reactor.

I might be very wrong here if the actual behaviour really is intended.

[ghost]

@tbuehlmann seems like we're in agreement then (about Backends::Base#stop being the right thing to do).

What I proposed in the previous note was changes in Backends::Base#stop that will allow callers to call it safely without it terminating the reactor loop for any other users of it in that process space. (Thats where the whole notion of "ownership" of the loop comes in).

So, to clarify, what I'm proposing is this (see the backends/base.rb file)

# Allow for early run up of eventmachine.

if EventMachine.reactor_running?
  @my_reactor = false
  starter.call
else
  @my_reactor = true
  EventMachine.run(&starter)
end

Then later, during stop!

EventMachine.stop if EventMachine.reactor_running? and @my_reactor

This way thin will only shut down the EM loop if it thinks it owns it. If it was added to an existing EM loop, it will play nice (and not stop the reactor).

Did I misunderstand your previous comment? Did you mean to point out some flaw with this approach? (which is entirely possible, tbh)

[tbuehlmann]

Yeah, my point is a bit different. Consider you have 3 services running inside one reactor: Thin, a Websocket server and a message queue whatnot. Now the author starts Thin first, the other services after it. Stopping Thin will now, even with your idea, shutdown the reactor entirely because the user didn't know better.

The thing is that the user has to know about that fact and that's not good. I'd say: Just stop the backend and let the user quit the reactor itself.

[ghost]

In the situation you describe, the author would have to do something like this...

s = Thin::Server.new opts
w = Websocket::Serve.new
m = Amqp::Server.new

EM.run do
  s.start
  w.start
  m.start
end

As you can see, the user is in charge of the EM loop, and none of the services will shut it down.

OTOH with your approach, thin will have to somehow know not to bring its reactor down in some cases.

To phrase it another way - i agree with you about there being a need for thin not to bring down its rector, but i think that in thise cases, if the user is taking on the responsibility of stopping rhe reactor loop, the he/she should also take on the responsibility of starting it. After all, at that point we can assume that the user is aware of the underlying reactor.

What this avoids is the complication that arises for users of vanilla thin, who don't know/care about the reactor loop. In that case thin will silently bring up (and down) the reactor as it starts/exits. The workflow for that use case should be kept intact IMHO. (e.g. - see Thin::Runner - for when thin is invoked from the command line as a rails server)

[macournoyer]

I think checking if the event loop has already started like @AM4 pointed out is a good solution. Here's how I'd implement it.

@tbuehlmann you mention the case when there are several servers running. But they will all be running in the same loop which will be started BEFORE Thin. Thus Thin will not "own" the loop and will not stop it.

Let me know if you can try that branch and if it fixes your issue.

[ghost]

@macournoyer would you accept a patch for this so we can get it into 1.5.2 ? I'd like this in the next release because its affecting me as well.

[tbuehlmann]

If we assume that Thin won't start the EM loop, I'm fine with that. ;)

[macournoyer]

@AM4 oops! I forgot to paste the url: 55657a9.

[tbuehlmann]

Yup, that'll do.

[ghost]

sweet. thanks!