GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-95041: Fail syslog.syslog in case inner call to syslog.openlog fails by noamcohen97 · Pull Request #95264 · python/cpython

Conversation

@noamcohen97

@noamcohen97 noamcohen97 commented Jul 26, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: syslog issues #95041

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

bedevere-bot commented Jul 26, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@noamcohen97 noamcohen97 mentioned this pull request Jul 26, 2022
gh-95011: Migrate syslog module to Argument Clinic #95012
Merged
@noamcohen97
noamcohen97 force-pushed the gh-95041-openlog-fail branch from 7c6e493 to b9307e9 Compare July 26, 2022 06:49
@bedevere-bot

bedevere-bot commented Jul 26, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@noamcohen97
pythongh-95041: fail syslog.syslog in case inner call to syslog.openl…
8210ab3
…og fails
@noamcohen97
noamcohen97 force-pushed the gh-95041-openlog-fail branch from b9307e9 to 8210ab3 Compare July 26, 2022 06:52
@bedevere-bot

bedevere-bot commented Jul 26, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka
serhiy-storchaka reviewed Jul 26, 2022
View reviewed changes
Comment thread Modules/syslogmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/syslogmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@noamcohen97
return null if PyTuple_New() fails on syslog.syslog
c95b55c
@bedevere-bot

bedevere-bot commented Jul 26, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@noamcohen97
avoid using a pointer to freed memory on syslog.syslog
9813348
@noamcohen97
noamcohen97 force-pushed the gh-95041-openlog-fail branch from 9a20c80 to 9813348 Compare July 26, 2022 07:13
@bedevere-bot

bedevere-bot commented Jul 26, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error skip news needs backport to 3.10 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
serhiy-storchaka reviewed Jul 26, 2022
View reviewed changes
Comment thread Modules/syslogmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@noamcohen97
syslog.syslog: nicer error handling
b507a03
erlend-aasland
erlend-aasland reviewed Jul 26, 2022
View reviewed changes
Comment thread Modules/syslogmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@noamcohen97 @erlend-aasland
Update Modules/syslogmodule.c
5a097a6
Co-authored-by: Erlend Egeberg Aasland <[email protected]>
erlend-aasland
erlend-aasland approved these changes Jul 26, 2022
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, Noam!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Jul 26, 2022
@erlend-aasland

erlend-aasland commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka
serhiy-storchaka approved these changes Jul 26, 2022
View reviewed changes
@serhiy-storchaka

serhiy-storchaka commented Jul 26, 2022

Copy link
Copy Markdown
Member

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 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

miss-islington commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

miss-islington commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington miss-islington assigned serhiy-storchaka Jul 26, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 26, 2022
@noamcohen97 @miss-islington
pythongh-95041: Fail syslog.syslog in case inner call to syslog.openl…
a9c68d5
…og fails (pythonGH-95264)

(cherry picked from commit b1f648e)

Co-authored-by: Noam Cohen <[email protected]>
@bedevere-bot

bedevere-bot commented Jul 26, 2022

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 label Jul 26, 2022
@erlend-aasland 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
@miss-islington

miss-islington commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

bedevere-bot commented Jul 26, 2022

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 26, 2022
@noamcohen97 @miss-islington
pythongh-95041: Fail syslog.syslog in case inner call to syslog.openl…
5ae3cb1
…og fails (pythonGH-95264)

(cherry picked from commit b1f648e)

Co-authored-by: Noam Cohen <[email protected]>
@erlend-aasland erlend-aasland mentioned this pull request Jul 26, 2022
syslog issues #95041
Closed
@noamcohen97
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
@miss-islington @noamcohen97
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]>
@bedevere-bot

bedevere-bot commented Jul 26, 2022

Copy link
Copy Markdown

⚠️⚠️⚠️ 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:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. 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.
  4. Check if the failure is related to this commit (b1f648e) or if it is a false positive.
  5. 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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

The failure in test_concurrency seems unrelated to this PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington added a commit that referenced this pull request Jul 27, 2022
@miss-islington @noamcohen97
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 serhiy-storchaka removed their assignment Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@erlend-aasland erlend-aasland erlend-aasland approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news type-bug An unexpected behavior, bug, or error type-crash A hard crash of the interpreter, possibly with a core dump

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@noamcohen97 @bedevere-bot @erlend-aasland @serhiy-storchaka @miss-islington
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.