Skip to content

bpo-41993: Fix possible issues in remove_module()#22631

Merged
serhiy-storchaka merged 3 commits into
python:masterfrom
serhiy-storchaka:import-remove_module
Oct 11, 2020
Merged

bpo-41993: Fix possible issues in remove_module()#22631
serhiy-storchaka merged 3 commits into
python:masterfrom
serhiy-storchaka:import-remove_module

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Oct 10, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.8 labels Oct 10, 2020
vstinner
vstinner reviewed Oct 11, 2020
View reviewed changes
Comment thread Python/import.c
vstinner
vstinner approved these changes Oct 11, 2020
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@serhiy-storchaka
serhiy-storchaka merged commit 8287aad into python:master Oct 11, 2020
@miss-islington

miss-islington commented Oct 11, 2020

Copy link
Copy Markdown
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

@serhiy-storchaka
serhiy-storchaka deleted the import-remove_module branch October 11, 2020 13:51
@bedevere-bot

bedevere-bot commented Oct 11, 2020

Copy link
Copy Markdown

GH-22647 is a backport of this pull request to the 3.9 branch.

@miss-islington

miss-islington commented Oct 11, 2020

Copy link
Copy Markdown
Contributor

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 8287aadb75f6bd0154996424819334cd3839707c 3.8

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 11, 2020
@serhiy-storchaka @miss-islington
bpo-41993: Fix possible issues in remove_module() (pythonGH-22631)
bc000ae 
* PyMapping_HasKey() is not safe because it silences all exceptions and can return incorrect result.
* Informative exceptions from PyMapping_DelItem() are overridden with RuntimeError and
  the original exception raised before calling remove_module() is lost.
* There is a race condition between PyMapping_HasKey() and PyMapping_DelItem().
(cherry picked from commit 8287aad)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@miss-islington

miss-islington commented Oct 11, 2020

Copy link
Copy Markdown
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Oct 11, 2020

Copy link
Copy Markdown
Contributor

Sorry @serhiy-storchaka, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 8287aadb75f6bd0154996424819334cd3839707c 3.8

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Oct 11, 2020
@serhiy-storchaka
[3.8] bpo-41993: Fix possible issues in remove_module() (pythonGH-22631)
6c58859 
* PyMapping_HasKey() is not safe because it silences all exceptions and can return incorrect result.
* Informative exceptions from PyMapping_DelItem() are overridden with RuntimeError and
  the original exception raised before calling remove_module() is lost.
* There is a race condition between PyMapping_HasKey() and PyMapping_DelItem()..
(cherry picked from commit 8287aad)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-bot

bedevere-bot commented Oct 11, 2020

Copy link
Copy Markdown

GH-22649 is a backport of this pull request to the 3.8 branch.

serhiy-storchaka added a commit that referenced this pull request Oct 14, 2020
@miss-islington @serhiy-storchaka
bpo-41993: Fix possible issues in remove_module() (GH-22631) (GH-22647)
391a544 
* PyMapping_HasKey() is not safe because it silences all exceptions and can return incorrect result.
* Informative exceptions from PyMapping_DelItem() are overridden with RuntimeError and
  the original exception raised before calling remove_module() is lost.
* There is a race condition between PyMapping_HasKey() and PyMapping_DelItem().
(cherry picked from commit 8287aad)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-bot

bedevere-bot commented Oct 14, 2020

Copy link
Copy Markdown

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

Hi! The buildbot s390x RHEL8 3.9 has failed when building commit 391a544.

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/490/builds/77) and take a look at the build logs.
  4. Check if the failure is related to this commit (391a544) 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/490/builds/77

Failed tests:

  • test_multiprocessing_spawn

Failed subtests:

  • test_put - test.test_multiprocessing_spawn.WithProcessesTestQueue

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

== Tests result: FAILURE then ENV CHANGED ==

410 tests OK.

10 slowest tests:

  • test_concurrent_futures: 3 min
  • test_capi: 2 min 34 sec
  • test_multiprocessing_spawn: 2 min 22 sec
  • test_peg_generator: 2 min 10 sec
  • test_unparse: 1 min 37 sec
  • test_tokenize: 1 min 36 sec
  • test_gdb: 1 min 32 sec
  • test_subprocess: 1 min 22 sec
  • test_multiprocessing_forkserver: 1 min 17 sec
  • test_lib2to3: 1 min 17 sec

1 test altered the execution environment:
test_asyncio

14 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_nis
test_ossaudiodev test_startfile test_tix test_tk test_ttk_guionly
test_winconsoleio test_winreg test_winsound test_zipfile64

1 re-run test:
test_multiprocessing_spawn

Total duration: 10 min 55 sec

Click to see traceback logs 
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/multiprocessing/spawn.py", line 116, in spawn_main
    exitcode = _main(fd, parent_sentinel)
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/multiprocessing/spawn.py", line 126, in _main
    self = reduction.pickle.load(from_parent)
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/multiprocessing/synchronize.py", line 110, in __setstate__
    self._semlock = _multiprocessing.SemLock._rebuild(*state)
FileNotFoundError: [Errno 2] No such file or directory


Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/test/_test_multiprocessing.py", line 938, in test_put
    self.assertEqual(queue_empty(queue), False)
AssertionError: True != False


Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/asyncio/sslproto.py", line 321, in __del__
    self.close()
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/asyncio/sslproto.py", line 316, in close
    self._ssl_protocol._start_shutdown()
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/asyncio/sslproto.py", line 590, in _start_shutdown
    self._abort()
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/asyncio/sslproto.py", line 731, in _abort
    self._transport.abort()
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/asyncio/selector_events.py", line 680, in abort
    self._force_close(None)
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/asyncio/selector_events.py", line 731, in _force_close
    self._loop.call_soon(self._call_connection_lost, exc)
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/asyncio/base_events.py", line 746, in call_soon
    self._check_closed()
  File "/home/dje/cpython-buildarea/3.9.edelsohn-rhel8-z/build/Lib/asyncio/base_events.py", line 510, in _check_closed
    raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed

xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
@serhiy-storchaka @xzy3-cdc
bpo-41993: Fix possible issues in remove_module() (pythonGH-22631)
38658f7 
* PyMapping_HasKey() is not safe because it silences all exceptions and can return incorrect result.
* Informative exceptions from PyMapping_DelItem() are overridden with RuntimeError and
  the original exception raised before calling remove_module() is lost.
* There is a race condition between PyMapping_HasKey() and PyMapping_DelItem().
@serhiy-storchaka serhiy-storchaka removed their assignment Dec 29, 2020
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@serhiy-storchaka @adorilson
bpo-41993: Fix possible issues in remove_module() (pythonGH-22631)
f969a1a 
* PyMapping_HasKey() is not safe because it silences all exceptions and can return incorrect result.
* Informative exceptions from PyMapping_DelItem() are overridden with RuntimeError and
  the original exception raised before calling remove_module() is lost.
* There is a race condition between PyMapping_HasKey() and PyMapping_DelItem().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
@brettcannon brettcannon Awaiting requested review from brettcannon
@encukou encukou Awaiting requested review from encukou
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently
@ncoghlan ncoghlan Awaiting requested review from ncoghlan
@warsaw warsaw Awaiting requested review from warsaw

Assignees

No one assigned

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@serhiy-storchaka @miss-islington @bedevere-bot @vstinner @the-knights-who-say-ni