gh-95041: Fail syslog.syslog in case inner call to syslog.openlog fails #95264

noamcohen97 · 2022-07-26T06:46:14Z

Issue: syslog issues #95041

bedevere-bot · 2022-07-26T06:46:18Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-07-26T06:49:58Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

…og fails

bedevere-bot · 2022-07-26T06:52:16Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

Modules/syslogmodule.c

bedevere-bot · 2022-07-26T07:12:22Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-07-26T07:13:33Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

Modules/syslogmodule.c

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

erlend-aasland

LGTM. Thanks, Noam!

erlend-aasland · 2022-07-26T08:03:32Z

The only remaining issue, is that I'd like a NEWS entry that covers both this PR and Serhiy's PR. I don't think fixing this qualifies for skip news. Serhiy?

serhiy-storchaka · 2022-07-26T08:25:42Z

At first I thought that these are rather theoretical bugs that do not manifest themselves in the real world, or the effect of which is insignificant. Later I realized that it is possible that these bugs can lead to a crash if syslog.openlog(), syslog.syslog() and syslog.closelog() are used in a multi-threaded program. And I was in such a hurry to merge #95058 to unlock #95011 that I forgot to add a NEWS entry. I still does not have a reproducer to confirm that it is a realistic danger. Maybe I'll add a NEWS entry later, maybe with some tests, if they are not too burdensome.

erlend-aasland · 2022-07-26T08:47:13Z

Ok, I'm fine with that. Let's land this :)

miss-islington · 2022-07-26T11:34:18Z

Thanks @noamcohen97 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

miss-islington · 2022-07-26T11:34:21Z

Sorry @noamcohen97 and @serhiy-storchaka, I had trouble checking out the 3.11 backport branch.
Please backport using cherry_picker on command line.
cherry_picker b1f648efc56ff17e18ec2b7402d59a771b305004 3.11

…og fails (pythonGH-95264) (cherry picked from commit b1f648e) Co-authored-by: Noam Cohen <[email protected]>

bedevere-bot · 2022-07-26T11:34:29Z

GH-95275 is a backport of this pull request to the 3.10 branch.

miss-islington · 2022-07-26T11:38:58Z

Thanks @noamcohen97 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

bedevere-bot · 2022-07-26T11:39:05Z

GH-95277 is a backport of this pull request to the 3.11 branch.

…og fails (pythonGH-95264) (cherry picked from commit b1f648e) Co-authored-by: Noam Cohen <[email protected]>

…ls (GH-95264) (cherry picked from commit b1f648e) Co-authored-by: Noam Cohen <[email protected]>

bedevere-bot · 2022-07-26T13:47:41Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86 Gentoo Non-Debug with X 3.x has failed when building commit b1f648e.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/58/builds/2801) and take a look at the build logs.
Check if the failure is related to this commit (b1f648e) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/58/builds/2801

Summary of the results of the build (if available):

== Tests result: ENV CHANGED ==

424 tests OK.

10 slowest tests:

test_multiprocessing_spawn: 6 min 19 sec
test_tokenize: 4 min 8 sec
test_concurrent_futures: 4 min 6 sec
test_asyncio: 3 min 24 sec
test_unparse: 2 min 48 sec
test_lib2to3: 2 min 25 sec
test_multiprocessing_fork: 1 min 48 sec
test_multiprocessing_forkserver: 1 min 40 sec
test_gdb: 1 min 39 sec
test_venv: 1 min 31 sec

1 test altered the execution environment:
test_import

11 tests skipped:
test_dbm_ndbm test_devpoll test_ioctl test_kqueue test_launcher
test_msilib test_startfile test_winconsoleio test_winreg
test_winsound test_zipfile64

Total duration: 40 min 57 sec

Click to see traceback logs

TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok


TracebackTests.test_exec_failure) ... ok


Traceback (most recent call last):
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.nondebug/build/Lib/threading.py", line 1036, in _bootstrap_inner
    self.run()
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.nondebug/build/Lib/threading.py", line 973, in run
    self._target(*self._args, **self._kwargs)
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.nondebug/build/Lib/test/test_import/__init__.py", line 471, in run
    sys.settrace(None)
RuntimeError: Cannot install a trace function while another trace function is being installed
k


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_syntax_error) ... ok

erlend-aasland · 2022-07-26T13:54:12Z

The failure in test_concurrency seems unrelated to this PR.

…ls (GH-95264) (cherry picked from commit b1f648e) Co-authored-by: Noam Cohen <[email protected]>

bedevere-bot added the awaiting review label Jul 26, 2022

noamcohen97 mentioned this pull request Jul 26, 2022

gh-95011: Migrate syslog module to Argument Clinic #95012

Merged

noamcohen97 force-pushed the gh-95041-openlog-fail branch from 7c6e493 to b9307e9 Compare July 26, 2022 06:49

pythongh-95041: fail syslog.syslog in case inner call to syslog.openl…

8210ab3

…og fails

noamcohen97 force-pushed the gh-95041-openlog-fail branch from b9307e9 to 8210ab3 Compare July 26, 2022 06:52

serhiy-storchaka reviewed Jul 26, 2022

View reviewed changes

Modules/syslogmodule.c Outdated Show resolved Hide resolved

Modules/syslogmodule.c Show resolved Hide resolved

return null if PyTuple_New() fails on syslog.syslog

c95b55c

avoid using a pointer to freed memory on syslog.syslog

9813348

noamcohen97 force-pushed the gh-95041-openlog-fail branch from 9a20c80 to 9813348 Compare July 26, 2022 07:13

serhiy-storchaka added type-bug An unexpected behavior, bug, or error skip news needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 26, 2022

serhiy-storchaka reviewed Jul 26, 2022

View reviewed changes

Modules/syslogmodule.c Outdated Show resolved Hide resolved

syslog.syslog: nicer error handling

b507a03

erlend-aasland reviewed Jul 26, 2022

View reviewed changes

Modules/syslogmodule.c Outdated Show resolved Hide resolved

Update Modules/syslogmodule.c

5a097a6

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

erlend-aasland approved these changes Jul 26, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Jul 26, 2022

serhiy-storchaka approved these changes Jul 26, 2022

View reviewed changes

serhiy-storchaka merged commit b1f648e into python:main Jul 26, 2022

bedevere-bot removed the awaiting merge label Jul 26, 2022

miss-islington assigned serhiy-storchaka Jul 26, 2022

bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 26, 2022

erlend-aasland added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Jul 26, 2022

bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 26, 2022

erlend-aasland mentioned this pull request Jul 26, 2022

syslog issues #95041

Closed

noamcohen97 deleted the gh-95041-openlog-fail branch July 26, 2022 11:50

miss-islington added a commit that referenced this pull request Jul 26, 2022

gh-95041: Fail syslog.syslog in case inner call to syslog.openlog fai…

2f9e47c

…ls (GH-95264) (cherry picked from commit b1f648e) Co-authored-by: Noam Cohen <[email protected]>

miss-islington added a commit that referenced this pull request Jul 27, 2022

gh-95041: Fail syslog.syslog in case inner call to syslog.openlog fai…

9640c4c

…ls (GH-95264) (cherry picked from commit b1f648e) Co-authored-by: Noam Cohen <[email protected]>

serhiy-storchaka removed their assignment Sep 25, 2022

Uh oh!

gh-95041: Fail syslog.syslog in case inner call to syslog.openlog fails #95264

gh-95041: Fail syslog.syslog in case inner call to syslog.openlog fails #95264

Uh oh!

Conversation

noamcohen97 commented Jul 26, 2022 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Jul 26, 2022

Uh oh!

bedevere-bot commented Jul 26, 2022

Uh oh!

bedevere-bot commented Jul 26, 2022

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Jul 26, 2022

Uh oh!

bedevere-bot commented Jul 26, 2022

Uh oh!

Uh oh!

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

erlend-aasland commented Jul 26, 2022

Uh oh!

serhiy-storchaka commented Jul 26, 2022

Uh oh!

erlend-aasland commented Jul 26, 2022

Uh oh!

miss-islington commented Jul 26, 2022

Uh oh!

miss-islington commented Jul 26, 2022

Uh oh!

bedevere-bot commented Jul 26, 2022

Uh oh!

miss-islington commented Jul 26, 2022

Uh oh!

bedevere-bot commented Jul 26, 2022

Uh oh!

bedevere-bot commented Jul 26, 2022

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

erlend-aasland commented Jul 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

noamcohen97 commented Jul 26, 2022 •

edited by bedevere-bot

Loading