Skip to content

Commit

Permalink
Fix AssertionError if subprocess exits after system time is set back.
Browse files Browse the repository at this point in the history
Fixes #281
Fixes #458
Fixes #533
Fixes #552
Fixes #618
  • Loading branch information
mnaberez committed Jun 13, 2015
1 parent ab771cd commit fe4edfd
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 13 deletions.
5 changes: 5 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@
- Fixed a bug introduced in 3.1.0 where ``supervisord`` could crash when
attempting to display a resource limit error.

- Fixed a bug where ``supervisord`` could crash with the message
``Assertion failed for processname: RUNNING not in STARTING`` if a time
change caused the last start time of the process to be in the future.
Thanks to Róbert Nagy, Sergey Leschenko, and samhair for the patches.

3.1.3 (2014-10-28)
------------------

Expand Down
26 changes: 16 additions & 10 deletions supervisor/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,15 @@ def finish(self, pid, sts):
self.laststop = now
processname = self.config.name

tooquickly = now - self.laststart < self.config.startsecs
if now > self.laststart:
too_quickly = now - self.laststart < self.config.startsecs
else:
too_quickly = False
self.config.options.logger.warn(
"process %r (%s) laststart time is in the future, don't "
"know how long process was running so assuming it did "
"not exit too quickly" % (self.config.name, self.pid))

exit_expected = es in self.config.exitcodes

if self.killing:
Expand All @@ -497,7 +505,7 @@ def finish(self, pid, sts):
self._assertInState(ProcessStates.STOPPING)
self.change_state(ProcessStates.STOPPED)

elif tooquickly:
elif too_quickly:
# the program did not stay up long enough to make it to RUNNING
# implies STARTING -> BACKOFF
self.exitstatus = None
Expand All @@ -508,18 +516,16 @@ def finish(self, pid, sts):

else:
# this finish was not the result of a stop request, the
# program was in the RUNNING state but exited implies
# RUNNING -> EXITED
# program was in the RUNNING state but exited
# implies RUNNING -> EXITED normally but see next comment
self.delay = 0
self.backoff = 0
self.exitstatus = es

if self.state == ProcessStates.STARTING: # pragma: no cover
# XXX I don't know under which circumstances this
# happens, but in the wild, there is a transition that
# subverts the RUNNING state (directly from STARTING
# to EXITED), so we perform the correct transition
# here.
# if the process was STARTING but a system time change causes
# self.laststart to be in the future, the normal STARTING->RUNNING
# transition can be subverted so we perform the transition here.
if self.state == ProcessStates.STARTING:
self.change_state(ProcessStates.RUNNING)

self._assertInState(ProcessStates.RUNNING)
Expand Down
80 changes: 77 additions & 3 deletions supervisor/tests/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ def test_signal_error(self):
event = L[0]
self.assertEqual(event.__class__, events.ProcessStateUnknownEvent)

def test_finish(self):
def test_finish_stopping_state(self):
options = DummyOptions()
config = DummyPConfig(options, 'notthere', '/notthere',
stdout_logfile='/tmp/foo')
Expand Down Expand Up @@ -975,7 +975,7 @@ def test_finish(self):
self.assertEqual(event.extra_values, [('pid', 123)])
self.assertEqual(event.from_state, ProcessStates.STOPPING)

def test_finish_expected(self):
def test_finish_running_state_exit_expected(self):
options = DummyOptions()
config = DummyPConfig(options, 'notthere', '/notthere',
stdout_logfile='/tmp/foo')
Expand Down Expand Up @@ -1007,7 +1007,44 @@ def test_finish_expected(self):
self.assertEqual(event.extra_values, [('expected', True), ('pid', 123)])
self.assertEqual(event.from_state, ProcessStates.RUNNING)

def test_finish_tooquickly(self):
def test_finish_starting_state_laststart_in_future(self):
options = DummyOptions()
config = DummyPConfig(options, 'notthere', '/notthere',
stdout_logfile='/tmp/foo')
instance = self._makeOne(config)
instance.config.options.pidhistory[123] = instance
pipes = {'stdout':'','stderr':''}
instance.pipes = pipes
instance.config.exitcodes =[-1]
instance.laststart = time.time() + 3600 # 1 hour into the future
from supervisor.states import ProcessStates
from supervisor import events
instance.state = ProcessStates.STARTING
L = []
events.subscribe(events.ProcessStateExitedEvent, lambda x: L.append(x))
instance.pid = 123
instance.finish(123, 1)
self.assertEqual(instance.killing, 0)
self.assertEqual(instance.pid, 0)
self.assertEqual(options.parent_pipes_closed, pipes)
self.assertEqual(instance.pipes, {})
self.assertEqual(instance.dispatchers, {})
self.assertEqual(options.logger.data[0],
"process 'notthere' (123) laststart time is in the "
"future, don't know how long process was running so "
"assuming it did not exit too quickly")
self.assertEqual(options.logger.data[1],
'exited: notthere (terminated by SIGHUP; expected)')
self.assertEqual(instance.exitstatus, -1)
self.assertEqual(len(L), 1)
event = L[0]
self.assertEqual(event.__class__,
events.ProcessStateExitedEvent)
self.assertEqual(event.expected, True)
self.assertEqual(event.extra_values, [('expected', True), ('pid', 123)])
self.assertEqual(event.from_state, ProcessStates.RUNNING)

def test_finish_starting_state_exited_too_quickly(self):
options = DummyOptions()
config = DummyPConfig(options, 'notthere', '/notthere',
stdout_logfile='/tmp/foo', startsecs=10)
Expand Down Expand Up @@ -1037,6 +1074,43 @@ def test_finish_tooquickly(self):
self.assertEqual(event.__class__, events.ProcessStateBackoffEvent)
self.assertEqual(event.from_state, ProcessStates.STARTING)

def test_finish_running_state_laststart_in_future(self):
options = DummyOptions()
config = DummyPConfig(options, 'notthere', '/notthere',
stdout_logfile='/tmp/foo')
instance = self._makeOne(config)
instance.config.options.pidhistory[123] = instance
pipes = {'stdout':'','stderr':''}
instance.pipes = pipes
instance.config.exitcodes =[-1]
instance.laststart = time.time() + 3600 # 1 hour into the future
from supervisor.states import ProcessStates
from supervisor import events
instance.state = ProcessStates.RUNNING
L = []
events.subscribe(events.ProcessStateExitedEvent, lambda x: L.append(x))
instance.pid = 123
instance.finish(123, 1)
self.assertEqual(instance.killing, 0)
self.assertEqual(instance.pid, 0)
self.assertEqual(options.parent_pipes_closed, pipes)
self.assertEqual(instance.pipes, {})
self.assertEqual(instance.dispatchers, {})
self.assertEqual(options.logger.data[0],
"process 'notthere' (123) laststart time is in the "
"future, don't know how long process was running so "
"assuming it did not exit too quickly")
self.assertEqual(options.logger.data[1],
'exited: notthere (terminated by SIGHUP; expected)')
self.assertEqual(instance.exitstatus, -1)
self.assertEqual(len(L), 1)
event = L[0]
self.assertEqual(event.__class__,
events.ProcessStateExitedEvent)
self.assertEqual(event.expected, True)
self.assertEqual(event.extra_values, [('expected', True), ('pid', 123)])
self.assertEqual(event.from_state, ProcessStates.RUNNING)

def test_finish_with_current_event_sends_rejected(self):
from supervisor import events
L = []
Expand Down

0 comments on commit fe4edfd

Please sign in to comment.