logging.handlers.SysLogHandler doesn't get cleaned up properly on exit if it throws an exception

[ngie-eign]

BPO	47158
Nosy	@vsajip, @ngie-eign, @gst

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2022-03-29.23:56:50.825>
labels = ['type-bug', 'library', '3.9']
title = "logging.handlers.SysLogHandler doesn't get cleaned up properly on exit if it throws an exception"
updated_at = <Date 2022-03-31.15:30:09.356>
user = 'https://github.com/ngie-eign'

bugs.python.org fields:

activity = <Date 2022-03-31.15:30:09.356>
actor = 'ngie'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2022-03-29.23:56:50.825>
creator = 'ngie'
dependencies = []
files = []
hgrepos = []
issue_num = 47158
keywords = []
message_count = 8.0
messages = ['416309', '416317', '416318', '416344', '416413', '416427', '416430', '416441']
nosy_count = 3.0
nosy_names = ['vinay.sajip', 'ngie', 'gstarck']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue47158'
versions = ['Python 3.9']

[ngie-eign]

Something I noticed when trying to repro another issue:

% python
Python 3.8.13 (default, Mar 16 2022, 17:28:59)
[GCC 7.5.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import logging.handlers
>>> logging.handlers.SysLogHandler(address=("something-completely-bogus-doncha-know", 514))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.8/logging/handlers.py", line 829, in __init__
    ress = socket.getaddrinfo(host, port, 0, socktype)
  File "/usr/lib/python3.8/socket.py", line 918, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -2] Name or service not known
>>>
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/usr/lib/python3.8/logging/__init__.py", line 2127, in shutdown
    h.close()
  File "/usr/lib/python3.8/logging/handlers.py", line 892, in close
    self.socket.close()
AttributeError: 'SysLogHandler' object has no attribute 'socket'
% python3.9
Python 3.9.11 (main, Mar 16 2022, 17:27:06)
[GCC 7.5.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import logging.handlers
>>> logging.handlers.SysLogHandler(address=("something-completely-bogus-doncha-know", 514))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.9/logging/handlers.py", line 873, in __init__
    ress = socket.getaddrinfo(host, port, 0, socktype)
  File "/usr/lib/python3.9/socket.py", line 954, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -2] Name or service not known
>>>
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/usr/lib/python3.9/logging/__init__.py", line 2142, in shutdown
    h.close()
  File "/usr/lib/python3.9/logging/handlers.py", line 936, in close
    self.socket.close()
AttributeError: 'SysLogHandler' object has no attribute 'socket'

This is happening because logging.Handler is calling logging._addHandlerRef in logging.Handler.__init__ and _removeHandlerRef at exit via logging.shutdown(..).

[gst]

I think this is fixed in main, thanks to this:

https://github.com/python/cpython/blob/main/Lib/logging/handlers.py#L862-L863

[ngie-eign]

Yup! Looks like it! Just needs a back port.

[gst]

I guess it could be done relatively easily.

but the question is more: should it be done ?

[ngie-eign]

Grégory: good question.

I would personally advocate for doing it out of selfish interests.

I'm working with middleware based on 3.8 (moving to 3.9+ is non-trivial), and we have a common fault scenario where the system breaks if logging.handlers.SysLogHandler is instantiated and the target host cannot be resolved, like seen in the first comment.

Backporting the changes you referenced would make addressing the above issue easier, since the logic in connect(..) was moved into its own routine.

[vsajip]

The 3.8 branch is security-fix-only now, I'm afraid. And I'm not sure it's worth backporting this.

[gst]

Enji : you can use this then:

In [6]: class Fixed(logging.handlers.SysLogHandler):
   ...:     def __init__(self, *a, **kw):
   ...:         self.socket = None
   ...:         super().__init__(*a, **kw)
   ...:     def close(self):
   ...:         if self.socket is None:
   ...:             return
   ...:         super().close()

that looks to be enough to prevent the issue.

[ngie-eign]

Grégory: that will fix this issue, but what I really need is some of the other changes, like moving the getaddrinfo logic into a separate route (connect).