Skip to content

Commit eb96a2a

Browse files
mnaberezmnaberez
committed
Fix AssertionError if subprocess exits after system time is set back.
Fixes #281 Fixes #458 Fixes #533 Fixes #552 Fixes #618
1 parent d86023b commit eb96a2a

File tree

3 files changed

+108
-20
lines changed

3 files changed

+108
-20
lines changed

‎CHANGES.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@
2323
- Fixed a bug introduced in 3.1.0 where ``supervisord`` could crash when
2424
attempting to display a resource limit error.
2525

26+
- Fixed a bug where ``supervisord`` could crash with the message
27+
``Assertion failed for processname: RUNNING not in STARTING`` if a time
28+
change caused the last start time of the process to be in the future.
29+
Thanks to Róbert Nagy, Sergey Leschenko, and samhair for the patches.
30+
2631
- Errors from the web interface are now logged at the ``ERROR`` level.
2732
Previously, they were logged at the ``TRACE`` level and easily
2833
missed. Thanks to Thomas Güttler for reporting this issue.

‎supervisor/process.py

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,15 @@ def finish(self, pid, sts):
453453
self.laststop = now
454454
processname = self.config.name
455455

456-
tooquickly = now - self.laststart < self.config.startsecs
456+
if now > self.laststart:
457+
too_quickly = now - self.laststart < self.config.startsecs
458+
else:
459+
too_quickly = False
460+
self.config.options.logger.warn(
461+
"process %r (%s) laststart time is in the future, don't "
462+
"know how long process was running so assuming it did "
463+
"not exit too quickly" % (self.config.name, self.pid))
464+
457465
exit_expected = es in self.config.exitcodes
458466

459467
if self.killing:
@@ -467,7 +475,7 @@ def finish(self, pid, sts):
467475
self._assertInState(ProcessStates.STOPPING)
468476
self.change_state(ProcessStates.STOPPED)
469477

470-
elif tooquickly:
478+
elif too_quickly:
471479
# the program did not stay up long enough to make it to RUNNING
472480
# implies STARTING -> BACKOFF
473481
self.exitstatus = None
@@ -478,18 +486,16 @@ def finish(self, pid, sts):
478486

479487
else:
480488
# this finish was not the result of a stop request, the
481-
# program was in the RUNNING state but exited implies
482-
# RUNNING -> EXITED
489+
# program was in the RUNNING state but exited
490+
# implies RUNNING -> EXITED normally but see next comment
483491
self.delay = 0
484492
self.backoff = 0
485493
self.exitstatus = es
486494

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

495501
self._assertInState(ProcessStates.RUNNING)

‎supervisor/tests/test_process.py

Lines changed: 87 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -898,24 +898,24 @@ def test_stopasgroup(self):
898898
self.assertEqual(event.extra_values, [('pid', 11)])
899899
self.assertEqual(event.from_state, ProcessStates.RUNNING)
900900

901-
def test_finish(self):
901+
def test_finish_stopping_state(self):
902902
options = DummyOptions()
903903
config = DummyPConfig(options, 'notthere', '/notthere',
904904
stdout_logfile='/tmp/foo')
905905
instance = self._makeOne(config)
906906
instance.waitstatus = (123, 1) # pid, waitstatus
907907
instance.config.options.pidhistory[123] = instance
908-
instance.killing = 1
908+
instance.killing = True
909909
pipes = {'stdout':'','stderr':''}
910910
instance.pipes = pipes
911911
from supervisor.states import ProcessStates
912912
from supervisor import events
913913
instance.state = ProcessStates.STOPPING
914914
L = []
915-
events.subscribe(events.ProcessStateStoppedEvent, lambda x: L.append(x))
915+
events.subscribe(events.ProcessStateEvent, lambda x: L.append(x))
916916
instance.pid = 123
917917
instance.finish(123, 1)
918-
self.assertEqual(instance.killing, 0)
918+
self.assertFalse(instance.killing)
919919
self.assertEqual(instance.pid, 0)
920920
self.assertEqual(options.parent_pipes_closed, pipes)
921921
self.assertEqual(instance.pipes, {})
@@ -929,7 +929,7 @@ def test_finish(self):
929929
self.assertEqual(event.extra_values, [('pid', 123)])
930930
self.assertEqual(event.from_state, ProcessStates.STOPPING)
931931

932-
def test_finish_expected(self):
932+
def test_finish_running_state_exit_expected(self):
933933
options = DummyOptions()
934934
config = DummyPConfig(options, 'notthere', '/notthere',
935935
stdout_logfile='/tmp/foo')
@@ -942,10 +942,10 @@ def test_finish_expected(self):
942942
from supervisor import events
943943
instance.state = ProcessStates.RUNNING
944944
L = []
945-
events.subscribe(events.ProcessStateExitedEvent, lambda x: L.append(x))
945+
events.subscribe(events.ProcessStateEvent, lambda x: L.append(x))
946946
instance.pid = 123
947947
instance.finish(123, 1)
948-
self.assertEqual(instance.killing, 0)
948+
self.assertFalse(instance.killing)
949949
self.assertEqual(instance.pid, 0)
950950
self.assertEqual(options.parent_pipes_closed, pipes)
951951
self.assertEqual(instance.pipes, {})
@@ -961,7 +961,48 @@ def test_finish_expected(self):
961961
self.assertEqual(event.extra_values, [('expected', True), ('pid', 123)])
962962
self.assertEqual(event.from_state, ProcessStates.RUNNING)
963963

964-
def test_finish_tooquickly(self):
964+
def test_finish_starting_state_laststart_in_future(self):
965+
options = DummyOptions()
966+
config = DummyPConfig(options, 'notthere', '/notthere',
967+
stdout_logfile='/tmp/foo')
968+
instance = self._makeOne(config)
969+
instance.config.options.pidhistory[123] = instance
970+
pipes = {'stdout':'','stderr':''}
971+
instance.pipes = pipes
972+
instance.config.exitcodes =[-1]
973+
instance.laststart = time.time() + 3600 # 1 hour into the future
974+
from supervisor.states import ProcessStates
975+
from supervisor import events
976+
instance.state = ProcessStates.STARTING
977+
L = []
978+
events.subscribe(events.ProcessStateEvent, lambda x: L.append(x))
979+
instance.pid = 123
980+
instance.finish(123, 1)
981+
self.assertFalse(instance.killing)
982+
self.assertEqual(instance.pid, 0)
983+
self.assertEqual(options.parent_pipes_closed, pipes)
984+
self.assertEqual(instance.pipes, {})
985+
self.assertEqual(instance.dispatchers, {})
986+
self.assertEqual(options.logger.data[0],
987+
"process 'notthere' (123) laststart time is in the "
988+
"future, don't know how long process was running so "
989+
"assuming it did not exit too quickly")
990+
self.assertEqual(options.logger.data[1],
991+
'exited: notthere (terminated by SIGHUP; expected)')
992+
self.assertEqual(instance.exitstatus, -1)
993+
self.assertEqual(len(L), 2)
994+
event = L[0]
995+
self.assertEqual(event.__class__, events.ProcessStateRunningEvent)
996+
self.assertEqual(event.expected, True)
997+
self.assertEqual(event.extra_values, [('pid', 123)])
998+
self.assertEqual(event.from_state, ProcessStates.STARTING)
999+
event = L[1]
1000+
self.assertEqual(event.__class__, events.ProcessStateExitedEvent)
1001+
self.assertEqual(event.expected, True)
1002+
self.assertEqual(event.extra_values, [('expected', True), ('pid', 123)])
1003+
self.assertEqual(event.from_state, ProcessStates.RUNNING)
1004+
1005+
def test_finish_starting_state_exited_too_quickly(self):
9651006
options = DummyOptions()
9661007
config = DummyPConfig(options, 'notthere', '/notthere',
9671008
stdout_logfile='/tmp/foo', startsecs=10)
@@ -970,7 +1011,6 @@ def test_finish_tooquickly(self):
9701011
pipes = {'stdout':'','stderr':''}
9711012
instance.pipes = pipes
9721013
instance.config.exitcodes =[-1]
973-
import time
9741014
instance.laststart = time.time()
9751015
from supervisor.states import ProcessStates
9761016
from supervisor import events
@@ -979,7 +1019,7 @@ def test_finish_tooquickly(self):
9791019
events.subscribe(events.ProcessStateEvent, lambda x: L.append(x))
9801020
instance.pid = 123
9811021
instance.finish(123, 1)
982-
self.assertEqual(instance.killing, 0)
1022+
self.assertFalse(instance.killing)
9831023
self.assertEqual(instance.pid, 0)
9841024
self.assertEqual(options.parent_pipes_closed, pipes)
9851025
self.assertEqual(instance.pipes, {})
@@ -992,6 +1032,43 @@ def test_finish_tooquickly(self):
9921032
self.assertEqual(event.__class__, events.ProcessStateBackoffEvent)
9931033
self.assertEqual(event.from_state, ProcessStates.STARTING)
9941034

1035+
def test_finish_running_state_laststart_in_future(self):
1036+
options = DummyOptions()
1037+
config = DummyPConfig(options, 'notthere', '/notthere',
1038+
stdout_logfile='/tmp/foo')
1039+
instance = self._makeOne(config)
1040+
instance.config.options.pidhistory[123] = instance
1041+
pipes = {'stdout':'','stderr':''}
1042+
instance.pipes = pipes
1043+
instance.config.exitcodes =[-1]
1044+
instance.laststart = time.time() + 3600 # 1 hour into the future
1045+
from supervisor.states import ProcessStates
1046+
from supervisor import events
1047+
instance.state = ProcessStates.RUNNING
1048+
L = []
1049+
events.subscribe(events.ProcessStateEvent, lambda x: L.append(x))
1050+
instance.pid = 123
1051+
instance.finish(123, 1)
1052+
self.assertFalse(instance.killing)
1053+
self.assertEqual(instance.pid, 0)
1054+
self.assertEqual(options.parent_pipes_closed, pipes)
1055+
self.assertEqual(instance.pipes, {})
1056+
self.assertEqual(instance.dispatchers, {})
1057+
self.assertEqual(options.logger.data[0],
1058+
"process 'notthere' (123) laststart time is in the "
1059+
"future, don't know how long process was running so "
1060+
"assuming it did not exit too quickly")
1061+
self.assertEqual(options.logger.data[1],
1062+
'exited: notthere (terminated by SIGHUP; expected)')
1063+
self.assertEqual(instance.exitstatus, -1)
1064+
self.assertEqual(len(L), 1)
1065+
event = L[0]
1066+
self.assertEqual(event.__class__,
1067+
events.ProcessStateExitedEvent)
1068+
self.assertEqual(event.expected, True)
1069+
self.assertEqual(event.extra_values, [('expected', True), ('pid', 123)])
1070+
self.assertEqual(event.from_state, ProcessStates.RUNNING)
1071+
9951072
def test_finish_with_current_event_sends_rejected(self):
9961073
from supervisor import events
9971074
L = []

0 commit comments

Comments
 (0)