Fixes for shutdown sequence, including prevention of a race condition

* Throw an exception if any events, runnables or messages are posted after shutdown. These were previosuly being silently ignored. We really should know about them.
* Don't exit the connection manager thread until the object manager thread exits. This fixes a race condition where the wakeup time of the conmgr thread would determine if it shutdown before or after the object manager, meaning it would sometimes ignore messages around shutdown time.
* Add a shutdown constraint to make the client manager shutdown before the invoker/event/conmgr threads. This gives a single point in the shutdown sequence that applications can use as a constraint (TODO: shutdown groups?)
* Emit a log message for each shutdown method call. This could be very useful in production.

git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@5491 542714f4-19e9-0310-aa3c-eee0fc999fb1
This commit is contained in:
Jamie Doornbos
2008-11-03 22:20:16 +00:00
parent 0cb3330de7
commit 82d4f36c1a
4 changed files with 43 additions and 5 deletions
@@ -201,6 +201,10 @@ public class PresentsDObjectMgr
// from interface DObjectManager // from interface DObjectManager
public void postEvent (DEvent event) public void postEvent (DEvent event)
{ {
if (!_running) {
throw new IllegalStateException("Object manager has been shutdown");
}
// assign the event's id and append it to the queue // assign the event's id and append it to the queue
event.eventId = getNextEventId(true); event.eventId = getNextEventId(true);
_evqueue.append(event); _evqueue.append(event);
@@ -273,6 +277,10 @@ public class PresentsDObjectMgr
*/ */
public void postRunnable (Runnable unit) public void postRunnable (Runnable unit)
{ {
if (!_running) {
throw new IllegalStateException("Object manager has been shutdown");
}
// just append it to the queue // just append it to the queue
_evqueue.append(unit); _evqueue.append(unit);
} }
@@ -563,6 +571,15 @@ public class PresentsDObjectMgr
} }
} }
/**
* Tests if the event processing thread is still running. This is required by the
* {@link ConnectionManager} to ensure messages posted just before or during shutdown are sent.
*/
public synchronized boolean isRunning ()
{
return _running;
}
/** /**
* Processes a single unit from the queue. * Processes a single unit from the queue.
*/ */
@@ -790,11 +807,6 @@ public class PresentsDObjectMgr
} }
} }
protected synchronized boolean isRunning ()
{
return _running;
}
protected int getNextOid () protected int getNextOid ()
{ {
// look for the next unused oid. in theory if we had two billion objects, this would loop // look for the next unused oid. in theory if we had two billion objects, this would loop
@@ -158,6 +158,17 @@ public class PresentsServer
// initialize the time base services // initialize the time base services
TimeBaseProvider.init(_invmgr, _omgr); TimeBaseProvider.init(_invmgr, _omgr);
// Make the client manager shut down before the invoker and dobj threads. This will help
// application code to avoid long chains of shutdown constraints (e.g. msoy bureau manager).
// TODO: is this check needed?
if (_invoker instanceof PresentsInvoker) {
_shutmgr.addConstraint(
_clmgr, ShutdownManager.Constraint.RUNS_BEFORE, (PresentsInvoker)_invoker);
} else {
log.warning("Invoker is not a PresentsInvoker", "invoker", _invoker);
}
} }
/** /**
@@ -127,6 +127,7 @@ public class ShutdownManager
// shut down all shutdown participants // shut down all shutdown participants
downers.apply(new ObserverList.ObserverOp<Shutdowner>() { downers.apply(new ObserverList.ObserverOp<Shutdowner>() {
public boolean apply (Shutdowner downer) { public boolean apply (Shutdowner downer) {
log.info("Calling shutdown registrant", "downer", downer);
downer.shutdown(); downer.shutdown();
return true; return true;
} }
@@ -255,6 +255,13 @@ public class ConnectionManager extends LoopingThread
report.append(avgOut).append(" avg size, "); report.append(avgOut).append(" avg size, ");
report.append(bytesOut*1000/sinceLast).append(" bps\n"); report.append(bytesOut*1000/sinceLast).append(" bps\n");
} }
@Override // from LoopingThread
public boolean isRunning ()
{
// Prevent exiting our thread until the object manager is done.
return super.isRunning() || _omgr.isRunning();
}
/** /**
* Notifies the connection observers of a connection event. Used internally. * Notifies the connection observers of a connection event. Used internally.
@@ -722,6 +729,9 @@ public class ConnectionManager extends LoopingThread
sendOutgoingMessages(System.currentTimeMillis()); sendOutgoingMessages(System.currentTimeMillis());
// unbind our listening socket // unbind our listening socket
// Note: because we wait for the object manager to exit before we do, we will still be
// accepting connections as long as there are events pending.
// TODO: consider shutting down the listen socker earlier, like in the shutdown method
try { try {
_ssocket.socket().close(); _ssocket.socket().close();
} catch (IOException ioe) { } catch (IOException ioe) {
@@ -846,6 +856,10 @@ public class ConnectionManager extends LoopingThread
*/ */
void postMessage (Connection conn, DownstreamMessage msg) void postMessage (Connection conn, DownstreamMessage msg)
{ {
if (!isRunning()) {
throw new IllegalStateException("Connection manager has been shutdown");
}
// sanity check // sanity check
if (conn == null || msg == null) { if (conn == null || msg == null) {
log.warning("postMessage() bogosity", "conn", conn, "msg", msg, new Exception()); log.warning("postMessage() bogosity", "conn", conn, "msg", msg, new Exception());