classification
Title: An infinite loop happens when we use SysLogHandler with eventlet
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Kazutaka.Morita, pitrou, python-dev, vinay.sajip
Priority: normal Keywords: patch

Created on 2012-06-25 11:29 by Kazutaka.Morita, last changed 2012-06-25 22:20 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
syslog.patch Kazutaka.Morita, 2012-06-25 11:41 fix a close leak in SysLogHandler review
Messages (4)
msg163939 - (view) Author: Kazutaka Morita (Kazutaka.Morita) Date: 2012-06-25 11:29
If we stop a syslog daemon when running the following program, it
leads to an infinite loop.

==
#!/usr/bin/env python

import eventlet
from logging.handlers import SysLogHandler
import time
import logging

eventlet.patcher.monkey_patch(all=False, socket=True)

logger = logging.getLogger('log')
logger.addHandler(SysLogHandler('/dev/log'))

while True:
    print "send a message to logger"
    logger.error("syslog test")
    time.sleep(1)
==

It is because there is a close leak in the python logging module, and
SysLogHandler continues to send a log message against the closed
connection forever.

The following patch seems to fix this problem:

diff -r 3b5545ba6432 Lib/logging/handlers.py
--- a/Lib/logging/handlers.py	Wed Jun 13 22:15:26 2012 -0400
+++ b/Lib/logging/handlers.py	Mon Jun 25 20:27:46 2012 +0900
@@ -801,7 +801,11 @@
         except socket.error:
             self.socket.close()
             self.socket = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
-            self.socket.connect(address)
+            try:
+                self.socket.connect(address)
+            except socket.error:
+                self.socket.close()
+                raise
 
     def encodePriority(self, facility, priority):
         """
msg163977 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-06-25 15:06
Why should the socket need closing if the connect fails? This seems a flawed contract; either connect() should work (in which case the socket will need closing when it's finished with) or it should fail, not connect and not require a close call (as it isn't connected). Could this be a problem with eventlet? One could argue that the leak is actually in whatever implements socket.connect in this case (presumably eventlet).
msg163981 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-06-25 15:30
> This seems a flawed contract; either connect() should work (in which
> case the socket will need closing when it's finished with) or it should 
> fail, not connect and not require a close call (as it isn't connected)

socket.close() closes the file descriptor, not only the underlying network connection. Otherwise the file descriptor will stay open until the socket object is garbage collected.
msg164020 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-06-25 22:20
New changeset 99f0c0207faa by Vinay Sajip in branch '2.7':
Issue #15179: Closed socket on connection failure. Thanks to Kazutaka Morita for the patch.
http://hg.python.org/cpython/rev/99f0c0207faa

New changeset 1bc1a14feb70 by Vinay Sajip in branch '3.2':
Issue #15179: Closed socket on connection failure. Thanks to Kazutaka Morita for the patch.
http://hg.python.org/cpython/rev/1bc1a14feb70

New changeset 6af0535b5e3a by Vinay Sajip in branch 'default':
Closes #15179: Merged fix from 3.2.
http://hg.python.org/cpython/rev/6af0535b5e3a
History
Date User Action Args
2012-06-25 22:20:39python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg164020

resolution: fixed
stage: resolved
2012-06-25 15:30:16pitrousetnosy: + pitrou
messages: + msg163981
2012-06-25 15:06:19vinay.sajipsetmessages: + msg163977
2012-06-25 13:10:52r.david.murraysetnosy: + vinay.sajip

versions: + Python 3.2
2012-06-25 11:41:38Kazutaka.Moritasethgrepos: - hgrepo137
2012-06-25 11:41:14Kazutaka.Moritasetfiles: + syslog.patch
hgrepos: + hgrepo137
keywords: + patch
2012-06-25 11:29:31Kazutaka.Moritacreate