HotSwapHandler race condition

[elipsion]

Jetty version(s)
All current

Java version/vendor
N/A

OS type/version
N/A

Description
setHandler() in HotSwapHandler has a race condition where the current handler is stopped before the new one is bound, because it uses updateBean(). This could lead to requests being dropped during the swap operation. A less racy implementation would look something like this, more or less inlining updateBean():

oldHandler = _handler
if(handler == oldHandler)
  return
if(handler != null) {
  handler.setServer(getServer())
  addBean(handler, true)
  if(isStarted() && !handler.isRunning()) // ContainerLifecycle does not start our handler if we're fully started
    handler.start()
}
_handler = handler
if(oldHandler != null)
  removeBean(oldHandler)

How to reproduce?
Bug found through static analysis of HotSwapHandler and ContainerLifeCycle

[gregw]

Thanks, Good catch.

However, I'm wondering about if we should always start the swapped handler? Perhaps we start it only if the handler we are replacing was started?

[elipsion]

I think the child handler should always be started if the HotSwapHandler itself is started (or starting).

I'd rather err on the side of caution and have started one too many as opposed to dropping requests on the floor because we didn't.