SystemError on importing module from unloaded package

[serhiy-storchaka]

BPO	30876
Nosy	@brettcannon, @doko42, @ncoghlan, @pitrou, @larryhastings, @ned-deily, @encukou, @ericsnowcurrently, @serhiy-storchaka, @vaultah
PRs	bpo-30876: Relative import from unloaded package now reimports the package #2639 bpo-30891: importlib _find_and_load() try/except #2665 [3.6] bpo-30876: Relative import from unloaded package now reimports the package (GH-2639) #2676 [3.5] bpo-30876: Relative import from unloaded package now reimports the package (GH-2639) #2677 bpo-30814, bpo-30876: Add new import test files to projects. #2851 [3.6] bpo-30814, bpo-30876: Add new import test files to projects. (GH-2851). #2912 [3.5] bpo-30876: Add new import test files to projects. (GH-2851). #2913
Files	import-systemerror.zip

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-07-27.10:18:26.840>
created_at = <Date 2017-07-08.05:07:16.228>
labels = ['interpreter-core', 'type-bug', '3.7']
title = 'SystemError on importing module from unloaded package'
updated_at = <Date 2017-08-09.03:19:53.605>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2017-08-09.03:19:53.605>
actor = 'larry'
assignee = 'none'
closed = True
closed_date = <Date 2017-07-27.10:18:26.840>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2017-07-08.05:07:16.228>
creator = 'serhiy.storchaka'
dependencies = []
files = ['46997']
hgrepos = []
issue_num = 30876
keywords = []
message_count = 28.0
messages = ['297940', '297942', '297943', '297944', '297947', '297964', '297965', '297986', '297992', '298193', '298194', '298196', '298200', '298201', '298417', '298889', '298950', '299231', '299251', '299297', '299303', '299304', '299828', '299832', '299833', '299871', '299963', '299964']
nosy_count = 10.0
nosy_names = ['brett.cannon', 'doko', 'ncoghlan', 'pitrou', 'larry', 'ned.deily', 'petr.viktorin', 'eric.snow', 'serhiy.storchaka', 'vaultah']
pr_nums = ['2639', '2665', '2676', '2677', '2851', '2912', '2913']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue30876'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[serhiy-storchaka]

It is possible to get SystemError on import (see attached archive).

$ ./python -c 'import package.module1'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/serhiy/py/cpython/package/module1.py", line 3, in <module>
    from . import module2
SystemError: Parent module 'package' not loaded, cannot perform relative import

SystemError means a programming error in interpreter core or extension. It is comparable to an assert in C code, but without immediate crashing. Since this situation can be provoked by user code, it should be replaced with other exception (KeyError, RuntimeError, ImportError, etc).

[ncoghlan]

I don't think we're that strict with SystemError - once folks are messing about with deleting things from the sys module, they *are* writing their own system level code, and may end up provoking SystemError if they corrupt the interpreter state in the process.

[ncoghlan]

To summarise what the attached source archive is doing, module1.py is essentially:

    import sys
    del sys.modules(__package__)
    from . import module2

So the only way to trigger this is by corrupting the import state, which seems like an appropriate use of SystemError to me.

[serhiy-storchaka]

I don't know other way to provoke SystemError by Python code. Always if SystemError was leaked this considered a bug and was fixed.

When unload package you need to remove its name and names of its submodules from sys.modules. This is a common case. If the submodule is imported at the same time in other thread you can get SystemError (randomly, with very small probability). I think that if the error can't be avoided, SystemError is a wrong exception for this case.

[ncoghlan]

If there are intermittent concurrent problems associated with this behaviour, I think that may be a sign that the current management of the per-module import locks is inadequate, since it isn't adequately accounting for direct manipulation of sys.modules in user code.

Fully resolving that would probably mean ensuring that:

1. For submodule imports, we always acquire the parent module locks in a consistent order, and then hang onto them until the submodule import is complete rather than letting them go as soon as the parent module import is complete
2. We move the current sys.modules to sys._modules, and make sys.modules a proxy mapping that acquires the relevant import locks in the same consistent order for set and delete operations before mutating sys._modules

If we did that, then we should consistently get one of the following two orders:

1. Thread A imports package, package.module1, package.module2; Thread B then unloads (and maybe reloads) package; or
2. Thread B then unloads (and maybe reloads) package; Thread A imports package, package.module1, package.module2

By contrast, at the moment there's a window where the following can happen:

• Thread A finishes importing "package" and starts importing "package.module1"
• Thread B unloads "package"
• Thread A hits SystemError through no fault of its own when it hits the "from . import module2" line

That said, formulating the problem that way does suggest another potential resolution: in this scenario, we could treat "from . import module2" *exactly* the same way we would handle "import package.module2", which would be to just import "package" again, rather than complaining that it "should" already be imported.

In addition to being vastly simpler to implement, that approach would have the virtue of also fixing the "del sys.modules(package)" case, not just the potential race condition.

[pitrou]

I concur with Serhiy: SystemError is a smell that C code isn't taking appropriate precautions before dealing with user code or data.

[brettcannon]

So this is very old semantics from the Python 2 days. I think Nick's idea of transforming the import to import pkg.mod when pkg isn't in sys.modules is probably the simplest, cleanest solution if we're going to change this.

[ncoghlan]

OK, so at least for 3.7, we'll replace the SystemError with a recursive import of the missing parent package, just as we'd expect to see with an absolute import.

I'm classing this as "Won't fix" for the native import system in 2.7 - folks wanting the fix there will need to switch to using importlib2 (assuming that gets updated to include the fix at some point).

That leaves 3.5 and 3.6, and I'd suggest we defer making a decision about whether or not to backport the fix to the maintenance branches until after we see how complex the patch for 3.7 ends up being.

[serhiy-storchaka]

It is easy to replace the SystemError with a recursive import of the missing parent package. It is enough to remove raising the SystemError.

The other effect of this change is that relative import from the top-level module now raises ImportError "attempted relative import with no known parent package" instead of SystemError "Parent module '' not loaded, cannot perform relative import".

[serhiy-storchaka]

New changeset 8a9cd20 by Serhiy Storchaka in branch 'master':
bpo-30876: Relative import from unloaded package now reimports the package (bpo-2639)
8a9cd20