python / cpython

The Python programming language
https://www.python.org
Other
62.67k stars 30.06k forks source link

implementations of the deprecated load_module import loader API, as prescribed by the documentation, are not thread safe #87824

Open e791423b-74a0-418d-81aa-baa9d4f185da opened 3 years ago

e791423b-74a0-418d-81aa-baa9d4f185da commented 3 years ago
BPO 43658
Nosy @kale-smoothie
Files
  • thread_unsafe_import.py: set env var PICK_API=N with N in {0,1} to demonstrate thread unsafe import and N=2 for expected behaviour
  • 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: ```python assignee = None closed_at = None created_at = labels = ['type-bug', '3.8', '3.9', '3.10', '3.7', 'docs'] title = 'implementations of the deprecated load_module import loader API, as prescribed by the documentation, are not thread safe' updated_at = user = 'https://github.com/kale-smoothie' ``` bugs.python.org fields: ```python activity = actor = 'kale-smoothie' assignee = 'docs@python' closed = False closed_date = None closer = None components = ['Documentation'] creation = creator = 'kale-smoothie' dependencies = [] files = ['49916'] hgrepos = [] issue_num = 43658 keywords = [] message_count = 1.0 messages = ['389713'] nosy_count = 2.0 nosy_names = ['docs@python', 'kale-smoothie'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue43658' versions = ['Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10'] ```

    e791423b-74a0-418d-81aa-baa9d4f185da commented 3 years ago

    Unless I've misread or misunderstood, the documentation at https://docs.python.org/3/reference/import.html#loaders for the deprecated load_module method doesn't indicate any requirements or caveats for thread safe importing. As it stands, I think it is not thread-safe, since the module is not protected against concurrent imports by the internal implementation marker __spec__._initializing = True.

    Additionally, the deprecated function decorator, importlib.util.module_for_loader seems to implement the marker incorrectly (sets __initializing__ directly on the module).

    I think this behaviour should either be documented as a major caveat, or internal details exposed to allow thread-safe implementations, or the old API removed entirely.