[issue38780] SysLogHandler crash atexit

Vinay Sajip added the comment: > As far as I know, this only happens during shutdown. During shutdown it has > already removed the attribute as part of the teardown process. In this case > adding the attribute at the begining will do no good. Certainly, that's true - there are a number of

[issue38780] SysLogHandler crash atexit

Alan Robertson added the comment: As far as I know, this only happens during shutdown. During shutdown it has already removed the attribute as part of the teardown process. In this case adding the attribute at the begining will do no good. On Wed, Jan 6, 2021, at 10:30 AM, Vinay Sajip

[issue38780] SysLogHandler crash atexit

Alan Robertson added the comment: On Wed, Jan 6, 2021, at 10:30 AM, Vinay Sajip wrote: > > Vinay Sajip added the comment: > > TBH as I said in the now-closed PR, using a NullSocket seems overkill. > As mentioned in msg359594, it seems to make more sense to assign a > socket attribute to

[issue38780] SysLogHandler crash atexit

Vinay Sajip added the comment: TBH as I said in the now-closed PR, using a NullSocket seems overkill. As mentioned in msg359594, it seems to make more sense to assign a socket attribute to None early in the constructor. If an error occurs during socket creation, the socket attribute will

[issue38780] SysLogHandler crash atexit

Erlend Egeberg Aasland added the comment: Using a NullSocket fixes the test in commit f0f8ff8f06afd43947e268824aa4381add05ce8f -- ___ Python tracker ___

[issue38780] SysLogHandler crash atexit

Erlend Egeberg Aasland added the comment: I've forked your work, Jason: https://github.com/erlend-aasland/cpython/tree/bpo-38780-test I added the test from msg356475, and I'm able to reproduce it by asserting that the `socket` attribute is present (commit

[issue38780] SysLogHandler crash atexit

Change by Jason R. Coombs : -- stage: patch review -> needs patch ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue38780] SysLogHandler crash atexit

Change by Jason R. Coombs : -- assignee: jaraco -> ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: I started work on a test in https://github.com/jaraco/cpython/tree/bugfix/bpo-38780-test, but (a) the test was failing to exhibit the expected failures, and (b) I realized that the fix isn't having the intended effect either, because for unix sockets,

[issue38780] SysLogHandler crash atexit

Erlend Egeberg Aasland added the comment: Thanks, Jason! I won't have time to look at this until perhaps next week, however, feel free to add tests as you like. N.B.: My patches might not be the best solution to the problem; it was just something I threw up based on my initial understanding

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: Erlend, inspired by your patches, I created https://github.com/python/cpython/pull/23661/commits/e9723003d49c722d57a69e5016b442d4d752fc6d, which uses a NullSocket instance instead of None, allowing the behavior-suppression to be encapsulated in a single

[issue38780] SysLogHandler crash atexit

Change by Jason R. Coombs : -- pull_requests: +22527 stage: -> patch review pull_request: https://github.com/python/cpython/pull/23661 ___ Python tracker ___

[issue38780] SysLogHandler crash atexit

Change by Jason R. Coombs : -- versions: +Python 3.10 -Python 2.7, Python 3.7 ___ Python tracker ___ ___ Python-bugs-list mailing

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: I've applied the patches and pushed them to https://github.com/jaraco/cpython/tree/bugfix/bpo-38780. ``` cpython master $ http https://bugs.python.org/file48933/0001-bpo-38780-Harden-socket-use-in-logging.handlers.patch | git apply cpython master $ git

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: I'll take a look at the patch and convert it to a PR. -- assignee: -> jaraco ___ Python tracker ___

[issue38780] SysLogHandler crash atexit

Alessio Bogon added the comment: Is there any update on this issue? I'm experiencing the same problem on macos. -- ___ Python tracker ___

[issue38780] SysLogHandler crash atexit

Change by Alessio Bogon : -- nosy: +youtux ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue38780] SysLogHandler crash atexit

Erlend Egeberg Aasland added the comment: I'm a bit uncertain if the previous patch can handle the emit() case correctly. Proposed improvement in attached patch (0002-Improve-emit.patch). -- Added file: https://bugs.python.org/file48934/0002-Improve-emit.patch

[issue38780] SysLogHandler crash atexit

Change by Erlend Egeberg Aasland : Added file: https://bugs.python.org/file48933/0001-bpo-38780-Harden-socket-use-in-logging.handlers.patch ___ Python tracker ___

[issue38780] SysLogHandler crash atexit

Change by Erlend Egeberg Aasland : Removed file: https://bugs.python.org/file48932/0001-bpo-38780-Harden-socket-use-in-logging.handlers.patch ___ Python tracker ___

[issue38780] SysLogHandler crash atexit

Erlend Egeberg Aasland added the comment: Attached patch is based on Ronald Oussoren's and Alan Robertson's comments: Initialise self.socket to None early in __init()__, and then check for None in close() and emit(). Passes make test on 3.9, 3.8 and 3.7. If this solution is ok I'll add a

[issue38780] SysLogHandler crash atexit

Alan Robertson added the comment: There are a variety of different reasons this can fail, not just on MacOS. You could give it a bad IP address of a server, etc. [That was my particular case]. The constructor should create an attribute 'socket' and initialize it to None early on. Then, the

[issue38780] SysLogHandler crash atexit

Ronald Oussoren added the comment: As you noted /dev/log doesn't exist on macOS (the correct path is /var/run/syslog). I guess the change that would be most in spirit of the comment is to set self.socket to None when the socket cannot be opened and test for that before closing the socket.

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: I could imagine extending the shutdown code to catch the reported error (https://github.com/python/cpython/blob/138ccbb02216ca086047c3139857fb44f3dab1f9/Lib/logging/__init__.py#L2130-L2135), but that wouldn't address the error in emit. Similarly, the

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: In the downstream issue, it's also reported that crashes occur in emit as well, suggesting that the comment is additionally wrong on that front. -- ___ Python tracker

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: The issue probably stems from https://github.com/python/cpython/blob/138ccbb02216ca086047c3139857fb44f3dab1f9/Lib/logging/handlers.py#L828-L835. I doubt the logic of that comment, as in the non-unix-socket case, the error is raised if a connection cannot

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: For background, this issue originated from https://github.com/pytest-dev/pytest-services/issues/20. -- ___ Python tracker ___

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: I guess that makes sense, as `/dev/log` doesn't exist on macOS. So maybe this usage is invalid. But still I'd expect the handler to error early or at least not error on shutdown. -- ___ Python tracker

[issue38780] SysLogHandler crash atexit

Change by Ned Deily : -- nosy: +vinay.sajip ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue38780] SysLogHandler crash atexit

Change by Jason R. Coombs : -- components: +Library (Lib), macOS nosy: +ned.deily, ronaldoussoren ___ Python tracker ___ ___

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: I only observe this issue on macOS. On Linux, the error doesn't occur. -- ___ Python tracker ___

[issue38780] SysLogHandler crash atexit

New submission from Jason R. Coombs : On Python 3.8.0: $ python -c "import logging.handlers, socket; handler = logging.handlers.SysLogHandler(facility=logging.handlers.SysLogHandler.LOG_LOCAL7, address='/dev/log', socktype=socket.SOCK_RAW)"

[issue38780] SysLogHandler crash atexit

Jason R. Coombs added the comment: Same error on Python 2.7 and 3.7 -- versions: +Python 2.7, Python 3.7, Python 3.8, Python 3.9 ___ Python tracker ___