Opened 15 years ago

Last modified 5 years ago

#2265 defect new

the reactor raises an exception when it receives a second SIGTERM or SIGINT

Reported by: titty Owned by: titty
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author:

Description

Using twisted svn, python 2.5

This program:

#! /usr/bin/env python

import os
import time
import signal

from twisted.internet import reactor

pid = os.getpid()

if os.fork():
    # parent
    reactor.run()
else:
    time.sleep(0.5)
    print "killing", pid
    os.kill(pid, signal.SIGTERM)
    os.kill(pid, signal.SIGINT)

produces the following error:

ralf@edgy:~/Twisted-trunk$ python t.py 
killing 10485
Traceback (most recent call last):
  File "t.py", line 13, in <module>
    reactor.run()
  File "/home/ralf/Twisted-trunk/twisted/internet/posixbase.py", line 218, in run
    self.mainLoop()
  File "/home/ralf/Twisted-trunk/twisted/internet/posixbase.py", line 226, in mainLoop
    self.runUntilCurrent()
--- <exception caught here> ---
  File "/home/ralf/Twisted-trunk/twisted/internet/base.py", line 561, in runUntilCurrent
    call.func(*call.args, **call.kw)
  File "/home/ralf/Twisted-trunk/twisted/internet/base.py", line 411, in _continueSystemEvent
    for callList in sysEvtTriggers[1], sysEvtTriggers[2]:
exceptions.TypeError: 'NoneType' object is unsubscriptable

Apparently multiple reactor.stop calls get scheduled from ReactorBase.sigInt/ReactorBase.sigBreak , but twisted does not handle them in any sane way.

I'm using the following workaround:

$ svn diff
Index: twisted/internet/base.py
===================================================================
--- twisted/internet/base.py	(revision 18814)
+++ twisted/internet/base.py	(working copy)
@@ -408,6 +408,9 @@
 
     def _continueSystemEvent(self, eventType):
         sysEvtTriggers = self._eventTriggers.get(eventType)
+        if not sysEvtTriggers:
+            return
+        
         for callList in sysEvtTriggers[1], sysEvtTriggers[2]:
             for callable, args, kw in callList:
                 try:

Change History (7)

comment:1 Changed 15 years ago by therve

With current trunk, the error became:

Traceback (most recent call last):
  File "2265.py", line 9, in ?
    reactor.run()
  File "/home/th/twisted_dev/twisted/trunk/twisted/internet/posixbase.py", line 221, in run
    self.mainLoop()
  File "/home/th/twisted_dev/twisted/trunk/twisted/internet/posixbase.py", line 229, in mainLoop
    self.runUntilCurrent()
--- <exception caught here> ---
  File "/home/th/twisted_dev/twisted/trunk/twisted/internet/base.py", line 641, in runUntilCurrent
    f(*a, **kw)
  File "/home/th/twisted_dev/twisted/trunk/twisted/internet/base.py", line 480, in stop
    raise RuntimeError, "can't stop reactor that isn't running"
exceptions.RuntimeError: can't stop reactor that isn't running

so the second stop just raise an exception. It's probably better than before, but I don't know if that's the best solution.

comment:2 Changed 15 years ago by titty

The following patch replaces sigInt, sigBreak, sigTerm by one signal handler, which calls a _safe_stop function. _safe_stop uses a flag set on the reactor instance to check if stop has already been called. It also checks the reactors running flag explicitly in case a signal arrives while the reactor is already shutting down.

ralf@red:~/twisted-trunk$ hg diff
diff -r 237cab1e5166 twisted/internet/base.py
--- a/twisted/internet/base.py  Fri Oct 26 11:57:19 2007 +0200
+++ b/twisted/internet/base.py  Fri Oct 26 16:42:11 2007 +0200
@@ -489,23 +489,21 @@ class ReactorBase(object):
         """
         self.running = False
 
-    def sigInt(self, *args):
-        """Handle a SIGINT interrupt.
+    def _handle_sig(self, signum, frame):
+        """signal handler, will shutdown the reactor
         """
-        log.msg("Received SIGINT, shutting down.")
-        self.callFromThread(self.stop)
+        log.msg("Received signal %s, shutting down." % (signum,))
+                
+        def _safe_stop():
+            if not getattr(self, '_handle_sig_flag', False):
+                self._handle_sig_flag = True
+                if not self.running:
+                    return
+                self.stop()
 
-    def sigBreak(self, *args):
-        """Handle a SIGBREAK interrupt.
-        """
-        log.msg("Received SIGBREAK, shutting down.")
-        self.callFromThread(self.stop)
+        self.callFromThread(_safe_stop)
 
-    def sigTerm(self, *args):
-        """Handle a SIGTERM interrupt.
-        """
-        log.msg("Received SIGTERM, shutting down.")
-        self.callFromThread(self.stop)
+    sigInt = sigBreak = sigTerm = _handle_sig
 
     def disconnectAll(self):
         """Disconnect every reader, and writer in the system.

comment:3 Changed 15 years ago by Jean-Paul Calderone

Adding another flag to the reactor is a bad idea. This just needs to be a simple state machine which handles its inputs properly.

Also, like, TDD.

comment:4 Changed 15 years ago by titty

I would have used the running flag, but I don't know enough about the shutdown event handling. I don't know how to check if a shutdown event has been triggered. So, I can't help here. At least the test program should be easy to convert to a unit test.

comment:5 Changed 15 years ago by Jean-Paul Calderone

The test program uses os.fork(), reactor.run(), and os.kill(). I don't think a unit test will be able to do any of those things. Not trying to be argumentative, just making sure if someone tries to work on this they don't start off in the wrong direction.

comment:6 Changed 13 years ago by Glyph

Owner: changed from Glyph to titty

comment:7 Changed 5 years ago by Glyph

Summary: signal delivery vs reactor.stopthe reactor raises an exception when it receives a second SIGTERM or SIGINT
Note: See TracTickets for help on using tickets.