[issue13487] inspect.getmodule fails when module imports change sys.modules

[Ned Deily]

Ned Deily  added the comment:


New changeset 7058d2d96c5ca4dfc6c754c5cd737c6eb2a8fd67 by Ned Deily (Miss 
Islington (bot)) in branch '3.7':
bpo-13487: Use sys.modules.copy() in inspect.getmodule() for thread safety. 
(GH-18786)
https://github.com/python/cpython/commit/7058d2d96c5ca4dfc6c754c5cd737c6eb2a8fd67


--
nosy: +ned.deily

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Gregory P. Smith]

Gregory P. Smith  added the comment:

If anyone else has a way to reproduce this issue, please run your tests with a 
3.7/3.8/3.9 interpreter with the fix I committed applied and report back if you 
still see this failure on that line.

I believe this to be fixed based on my own testing so I am closing it out.

--
assignee: eric.araujo -> gregory.p.smith
resolution:  -> fixed
stage: patch review -> commit review
status: open -> closed
versions: +Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[miss-islington]

miss-islington  added the comment:


New changeset 6b452ff97f70eca79ab956987cc04b6586feca00 by Miss Islington (bot) 
in branch '3.8':
bpo-13487: Use sys.modules.copy() in inspect.getmodule() for thread safety. 
(GH-18786)
https://github.com/python/cpython/commit/6b452ff97f70eca79ab956987cc04b6586feca00


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[miss-islington]

miss-islington  added the comment:


New changeset a12381233a243ba7d5151ebf060feb57dd540bef by Miss Islington (bot) 
in branch '3.7':
bpo-13487: Use sys.modules.copy() in inspect.getmodule() for thread safety. 
(GH-18786)
https://github.com/python/cpython/commit/a12381233a243ba7d5151ebf060feb57dd540bef


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Testing by changing list(sys.modules.items()) to sys.modules.copy().items() 
internally with a large integration test that was reliably flaky on this line 
before shows that the .copy().items() worked.  The test is reliable again.

So I've gone ahead and pushed those changes in.  PyDict_Copy()'s implementation 
at first ~5 minute glance did not appear to have calls to code I'd expect to 
re-enter Python releasing the GIL.  But I didn't try to do a deep dive.  It 
works for us and is logically equivalent.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset 85cf1d514b84dc9a4bcb40e20a12e1d82ff19f20 by Gregory P. Smith in 
branch 'master':
bpo-13487: Use sys.modules.copy() in inspect.getmodule() for thread safety. 
(GH-18786)
https://github.com/python/cpython/commit/85cf1d514b84dc9a4bcb40e20a12e1d82ff19f20


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 10.0 -> 11.0
pull_requests: +18144
pull_request: https://github.com/python/cpython/pull/18787

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[miss-islington]

Change by miss-islington :


--
pull_requests: +18145
pull_request: https://github.com/python/cpython/pull/18788

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Gregory P. Smith]

Change by Gregory P. Smith :


--
pull_requests: +18143
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/18786

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Serhiy: Why is dict.copy() not thread safe?

if what you say of list(dict) being safe, iterating over that and looking up 
the keys would work.  But all of this is entirely non-obvious to the reader of 
the code.  all of these _look_ like they should be safe.

We should make dict.copy() safe and document the guarantee as such as that one 
could at least be explained when used for that purpose.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Gregory P. Smith]

Gregory P. Smith  added the comment:

fyi - we just had a test run into this (in a flaky manner - definitely a race 
condition) at work:

```
...
for f in inspect.stack(context=0)
  File "/inspect.py", line 1499, in stack
return getouterframes(sys._getframe(1), context)
  File "/inspect.py", line 1476, in getouterframes
frameinfo = (frame,) + getframeinfo(frame, context)
  File "/inspect.py", line 1446, in getframeinfo
filename = getsourcefile(frame) or getfile(frame)
  File "/inspect.py", line 696, in getsourcefile
if getattr(getmodule(object, filename), '__loader__', None) is not None:
  File "/inspect.py", line 732, in getmodule
for modname, module in list(sys.modules.items()):
RuntimeError: dictionary changed size during iteration
```

We haven't diagnosed what was leading to it though.  Trust in the ability to 
use inspect.stack() -> ... -> inspect.getmodule() in multithreaded code is on 
the way out as a workaround.

(this was on 3.6.7)

A workaround we could checkin without consequences should be to change

list(sys.modules.items())  into  list(sys.modules.copy().items()).

I was a bit surprised to see this happen at all, list(dict.items()) seems like 
it should've been done entirely in C with the GIL held the entire time.  but 
maybe I'm just missing where the GIL would be released in the calls to exhause 
the iterator made by 
https://github.com/python/cpython/blob/master/Objects/listobject.c ?

--
nosy: +gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Patrik Simons]

Patrik Simons added the comment:

I cannot reproduce. In fact I cannot even get list(d.items())
to raise RuntimeError: dictionary changed size during iteration
for any dict d.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

dict.copy() is not thread-safe still (but it can be made thread-safe).

list(dict) is thread-safe. It copies a list of keys only.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Éric Araujo]

Éric Araujo added the comment:

Could you give code to reproducer the problem, if possible without third-party 
dependencies?

--
resolution: fixed -> 
status: closed -> open
versions: +Python 3.6, Python 3.7 -Python 3.2, Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Thomas Dudziak]

Thomas Dudziak added the comment:

This bug seems to be still there. I had an application fail with this same 
error in inspect.getouterframes with Python 3.6.2. As far as I could trace it, 
during iteration over sys.modules _tracemalloc and tracemalloc were added, not 
quite sure from where (maybe the warnings module ?)

--
nosy: +tomdzk

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Patrik Simons]

Patrik Simons added the comment:

list(sys.modules.items()) still raises RuntimeError: dictionary changed size 
during iteration when another thread imports a module.

I would assume dict.copy() is thread-safe so a working fix could use 
sys.modules.copy().items()

I hit this bug when printing the name of the caller function using
inspect.stack(). The workaround in this case is calling 
inspect.stack(context=0).

--
nosy: +psimons

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Éric Araujo]

Changes by Éric Araujo mer...@netwok.org:


--
assignee:  - eric.araujo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Roundup Robot]

Roundup Robot devn...@psf.upfronthosting.co.za added the comment:

New changeset 2ef359d7a2e9 by Éric Araujo in branch '3.2':
Fix inspect.getmodule to use a copy of sys.modules for iteration (#13487).
http://hg.python.org/cpython/rev/2ef359d7a2e9

--
nosy: +python-dev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Éric Araujo]

Éric Araujo mer...@netwok.org added the comment:

Committed, thanks.

--
resolution:  - fixed
stage: patch review - committed/rejected
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Éric Araujo]

Éric Araujo mer...@netwok.org added the comment:

Maybe it can be caused by an installation happening during the loop.  I agree 
with Erik’s reading of the comment and patch, and don’t think a test is needed.

--
nosy: +eric.araujo
stage:  - patch review
type:  - behavior
versions:  -Python 3.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Erik Tollerud]

Erik Tollerud erik.tolle...@gmail.com added the comment:

The package that triggers it for me is the py (http://pypi.python.org/pypi/py) 
package - when in gets imported, it does some trick with sys.modules that is in 
place to get around some pickling restriction, but that means sys.modules is 
altered during the import of the `py` package... and that triggers the 
exception.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc amaur...@gmail.com added the comment:

When a package is imported sys.modules changes... nothing special here.
But it's true true that py.std, for example, is a lazy module with a special 
__getattr__ that will import submodules.

Patch looks good to me as well.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Meador Inge]

Changes by Meador Inge mead...@gmail.com:


--
nosy: +meador.inge

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc amaur...@gmail.com added the comment:

You are certainly right, but I wonder how this can happen.
Are there modules which import something just by looking at them?
Or is is some race condition due to another running thread?

--
nosy: +amaury.forgeotdarc

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Erik Tollerud]

New submission from Erik Tollerud erik.tolle...@gmail.com:

The inspect.getmodule function crashes if packages are installed that futz with 
sys.modules while they are being tested for module status or the like.  I'm not 
actually sure which packages are doing this, but the symptom is the for loop 
over sys.modules raises an Exception because it is modified while the loop is 
running.

This is *not* a problem in Python 2.x because sys.modules.items() returns a 
copy of the dictionary instead of an iterator, and 3.x changes that behavior.  
The comment above the for loop makes it clear that the expected behavior is a 
copy rather than an iterator, so the attached patch corrects the problem by 
simply wrapping the items() call in list().

--
components: Library (Lib)
files: getmodulefix.patch
keywords: patch
messages: 148438
nosy: Erik.Tollerud
priority: normal
severity: normal
status: open
title: inspect.getmodule fails when module imports change sys.modules
versions: Python 3.1, Python 3.2, Python 3.3
Added file: http://bugs.python.org/file23790/getmodulefix.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13487] inspect.getmodule fails when module imports change sys.modules

[Eric Snow]

Changes by Eric Snow ericsnowcurren...@gmail.com:


--
nosy: +eric.snow

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13487
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com