classification
Title: test.test_imp.ImportTests.test_load_source has side effects
Type: behavior Stage: resolved
Components: Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Mariatta, brett.cannon, eric.snow, ezio.melotti, haypo, michael.foord, ncoghlan, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-10-03 12:46 by serhiy.storchaka, last changed 2017-10-18 08:13 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
tests-bigmem.txt serhiy.storchaka, 2017-10-03 12:47 Full log
Pull Requests
URL Status Linked Edit
PR 3871 merged haypo, 2017-10-03 14:42
PR 3988 merged python-dev, 2017-10-13 20:49
Messages (9)
msg303608 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-03 12:46
I have found a strange failure when run the bigmem tests.

0:20:19 load avg: 1.04 [116/407/1] test_cgitb
test test_cgitb failed -- Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_cgitb.py", line 23, in test_html
    raise ValueError("Hello World")
ValueError: Hello World

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_cgitb.py", line 27, in test_html
    html = cgitb.html(sys.exc_info())
  File "/home/serhiy/py/cpython/Lib/cgitb.py", line 117, in html
    records = inspect.getinnerframes(etb, context)
  File "/home/serhiy/py/cpython/Lib/inspect.py", line 1483, in getinnerframes
    frameinfo = (tb.tb_frame,) + getframeinfo(tb, context)
  File "/home/serhiy/py/cpython/Lib/inspect.py", line 1445, in getframeinfo
    lines, lnum = findsource(frame)
  File "/home/serhiy/py/cpython/Lib/inspect.py", line 780, in findsource
    module = getmodule(object, file)
  File "/home/serhiy/py/cpython/Lib/inspect.py", line 739, in getmodule
    f = getabsfile(module)
  File "/home/serhiy/py/cpython/Lib/inspect.py", line 708, in getabsfile
    _filename = getsourcefile(object) or getfile(object)
  File "/home/serhiy/py/cpython/Lib/inspect.py", line 693, in getsourcefile
    if os.path.exists(filename):
  File "/home/serhiy/py/cpython/Lib/genericpath.py", line 19, in exists
    os.stat(path)
ValueError: embedded null byte

0:20:19 load avg: 1.04 [117/407/2] test_csv -- test_cgitb failed


It is not reproduced when run test_cgitb separately.
msg303619 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-10-03 14:21
I bisect the bug manually and using test.bisect. I identified that the bug is triggered when you run the following 3 tests:
---
test.test_imp.ImportTests.test_load_source
test.test_cgitb.TestCgitb.test_html
test.test_cgitb.TestCgitb.test_fonts
---

Write these tests into "tests" file and run:

./python -m test.bisect -uall -M4G test_imp test_cgitb --matchfile=tests
msg303620 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-10-03 14:22
Sorry, run:

./python -m test -M4G test_imp test_cgitb --matchfile=tests
msg303621 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-10-03 14:31
The problem is that imp.load_source() modifies __file__ of the __main__ module.

Example of a file x.py:
---
import imp
import sys
print(sys.modules[__name__])
try:
    imp.load_source(__name__, __file__ + "\0")
except:
    pass
print(sys.modules[__name__])
---

Output:
---
<module '__main__' from 'x.py'>
<module '__main__' from 'x.py\x00'>
---
msg303622 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-10-03 14:44
Attached PR 3871 fixes the side effect in test_imp, but I'm not sure that it's the best fix. Maybe load_source() should use a "transaction" to restore attributes on failure?

load_source() seems to be used imp.reload() for example. Do you expect this function to restore the imported module to its original state on failure?
msg303713 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2017-10-04 16:54
The whole imp module is deprecated so I'm personally not bothered by imp.load_source() not being strengthened to be more sane.
msg304358 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-10-13 20:47
New changeset a505ecdc5013cd8f930aacc1ec4fb2afa62d3853 by Victor Stinner in branch 'master':
bpo-31676: Fix test_imp.test_load_source() side effect (#3871)
https://github.com/python/cpython/commit/a505ecdc5013cd8f930aacc1ec4fb2afa62d3853
msg304551 - (view) Author: Mariatta Wijaya (Mariatta) * (Python committer) Date: 2017-10-18 01:47
New changeset 178148025494c4058571831fb11fc8eeff8b7365 by Mariatta (Miss Islington (bot)) in branch '3.6':
bpo-31676: Fix test_imp.test_load_source() side effect (GH-3871) (GH-3988)
https://github.com/python/cpython/commit/178148025494c4058571831fb11fc8eeff8b7365
msg304560 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-10-18 08:13
I tested "./python -m test -uall -M4G test_imp test_cgitb" on 3.6 and the master branch. Both tests now pass correctly. I close the issue.
History
Date User Action Args
2017-10-18 08:13:52hayposetstatus: open -> closed
resolution: fixed
messages: + msg304560

stage: patch review -> resolved
2017-10-18 01:47:12Mariattasetnosy: + Mariatta
messages: + msg304551
2017-10-13 20:52:10hayposetversions: + Python 3.6
2017-10-13 20:49:11python-devsetpull_requests: + pull_request3964
2017-10-13 20:47:51hayposetmessages: + msg304358
2017-10-04 16:54:07brett.cannonsetmessages: + msg303713
2017-10-03 14:44:08hayposetmessages: + msg303622
2017-10-03 14:42:38hayposetkeywords: + patch
stage: patch review
pull_requests: + pull_request3849
2017-10-03 14:33:24hayposetnosy: + brett.cannon, ncoghlan, eric.snow
title: Strange failure in test_cgitb -> test.test_imp.ImportTests.test_load_source has side effects

versions: + Python 3.7
2017-10-03 14:31:32hayposetmessages: + msg303621
2017-10-03 14:22:01hayposetmessages: + msg303620
2017-10-03 14:21:03hayposetmessages: + msg303619
2017-10-03 12:47:37serhiy.storchakasetfiles: + tests-bigmem.txt
2017-10-03 12:46:07serhiy.storchakacreate