The second try to reimport a module after the interrupted first import is broken

Python Help
1

Hello,

Based on what user Chris Angelico described in Why do relative from-imports also add the submodule itself to the namespace?, we should normally have the submodule set after a from-import.

We are currently encounter an issue where this isn’t true, i.e the submodule isn’t guaranteed to be set after from ... import .... if the package is tried to be reimported the second time after a failed import.

The issue we have is described in apache/airflow#41359 where an import is interuptted the first time, then the second time the import doesn’t work as described (only explicit import is set in package’s namespace), we included an MCVE inside that topic if you want to do a quick test.

We don’t know why there is a different between a normal import versus a second try to reimport after being interrupted.

In that issue, we have a package redis-py with its __init.py__:

# redis/__init__.py
from redis.client import Redis, StrictRedis

There isn’t any other import for client inside that file.
In normal situation (e.g import redis from python shell), we see that redis.client is set, client is available in dir(redis).

But in rare case where the import took some time and be interrupted, in our case, it is interrupted by threading.Timer timeout in outer call (though redis & redis.* exist in sys.modules even after being interrupted), then the next time the import is called again and isn’t interrupted, the dir(redis) doesn’t contain redis.client or other “implicit” import. Maybe the first import has done the import but doesn’t set the “implicit” submodule in package’s namespace?

We could neither find any issue about this, nor understand where to look

Any help is appreciated!
Thanks!

2

This is somewhat expected, an exception during an import is going to leave the module in an arbitrary messed up state, you really, really should avoid getting into such a situation, and if you do, your best bet is to completely throw away the half imported modules.

The problem here is that simplified, the sequences is something like this:

def import_module(modulename):
    spec = find_spec(modulename)
    module = create_module_object(spec)
    sys.modules[modulename] =module
    exec_module(spec, module)

def import_submodule(base_modulename, sub_modulename):
    import_module(base_modulename)
    import_module(full_name := f'{base_modulename}.{sub_modulename}')
    setattr(sys.modules[base_modulename], sub_modulename, sys.modules[full_name])

(in truth this is a bit more complex ofcourse and doesn’t have these exact two functions anywhere. Read the full documentation of the import system)

The interrupt/exception you are talking about happens somewhere inside of exec_module for the submodule import, meaning the setattr call never happens.

When the import then happens again, none of this ever happens because full_name is already in sys.modules. This also means that the body of the module does not get properly executed either, meaning the module is an abitrary messed up half-initialized state.

With other words: If the import is interrupted, don’t try to use those modules. Best action is to restart the interpreter, or if that really, really isn’t an option, remove entries from sys.modules that might be in a half initalized state (which includes anything imported by anything, including stdlib modules that weren’t already imported)

2 Likes
3

Thank for your reply, Cornelius

What we don’t understand is that even if full_name and its submodule is already in sys.modules, we saw that the full_name is still executed anew during second import (we add print("In redis/__init__.py") and print("In redis/client.py") at the first line of each respective file with the provided MCVE in the github issue:

In redis/__init__.py
In redis/client.py
FunctionTimedOut

After failed import redis
'redis' in sys.modules: True
'redis.client' in sys.modules: True

Reimport mock_kombu_transport_redis
In redis/__init__.py
After reimport redis
'redis' in sys.modules: True
'redis.client' in sys.modules: True
'client' in dir(redis): False
getattr(redis, 'client', None)=None

After reimport redis.client
'client' in dir(redis): False
getattr(redis, 'client', None)=None

After reimport: from redis import client
client=<module 'redis.client' from '***/site-packages/redis/client.py'>
'client' in dir(redis): False
getattr(redis, 'client', None)=None
client.Pipeline=<class 'redis.client.Pipeline'>

If we executed a second time a module with success, why only the main module is executed but import_submodule is omitted the second time? I suppose there is a successful flag on each module? So even if a module exists in sys.modules, it could still be reloaded?

I’ve tried to read the import system at first, but didn’t catch this, I will try the doc again.

Furthermore, I get what you want to say about scratching the faulty import, it is logical, but if we are in production environment and we have several dynamic import not in our control, it is quite hard to know which one is messed up, we discover this case completely by chance, how should we find out every broken import? It is also hard to restart the whole system at will even if we discover the mess much later :sweat_smile:

4

IDK, this behavior surprises me. AFAIK, no execution should happen, will try to see if I can get a local reproducers without this much code.

AFAIK, no.

Via an explicit call to reload_module, yes. This might also be a viable way to fix up your broken modules, but I wouldn’t rely on it (since it creates e.g. new class objects).

:person_shrugging: IDK really. You maybe could hijack the import system by overwriting builtins.__import__ to prevent interrupts during an import statement, or at least do something like creating a list of all imported modules before the interrupt happen.

Alternatively, do some work to prevent dynamic imports and force all imports in your system to be global on interpreter startup.

But note that not handling this is a terrible idea, especially if you have third party C modules. It could cause segfaults by creating partially initialized C modules.

5

Oh, the proper loading code should be removing the entry for the failing import from sys.module. So it’s an interesting question when exactly the the timeout is raised, and how exactly that is done.

6

What we saw in the log in my last message is that both redis and redis.client exist in sys.modules after an interrupted import and just before the second re-import, the sleep we added at the end of redis/__init__.py so every submodules should be loaded correctly. We don’t understand why python still try to execute the main module if there is already a reference in sys.modules?

7

I see this as a simple issue of a bug in the design that allows the interruption to happen. Refactor the code to require a initialisation function that is called after the imports have all completed.

I am bases is it on a very quick scan of the bug ticket and the mention of timeouts.

I have used this pattern where initialisation depends on external resources, like network connects, to function,

8

Thank Barry,

Could you tell us what did you do inside the initialization function, is it used to do a healthy check and/or reload module?

We thought that if we have to import everything before actual main code, doesn’t it defeat the purpose of dynamic import where we want to load only the necessary module when it is needed? And if an import took too long, shouldn’t it be interruptible to not affect the whole system?

9

My guess is that the code in the module is doing two things.

  1. defining classes and functions
  2. running some code to do some setup

What I do is define the classes and functions but I do not run code as part of the module importing.

Any business code that needs running like connect to a database, connect to a cloud server etc is always done by calling a function AFTER the import.

You should never need to reimport a module.

The import will not be slow unless you run business logic as a side effect of import.

Once you put the business logic into functions that are called after import you are in control. Now a timeout is part of your API.

For example:

import myapp

try:
    myapp.setup()
except myapp.TimeoutError:
    print('Timeout!')
10

I don’t think that’s correct, actually — or at least, if it is correct, it’s only a temporary situation.

Don’t forget that, even though func_timeout() has returned, redis/__init__.py is still sleeping, so it’s not yet completed and been cleaned up.

Two things are informative:

  1. Running with python3 -v, so you can see imports happening as they go
  2. Adding some sleep()s to the mock_airflow.py code, to match the ones added to the redis/__init__.py code.

If you do those two things, i.e.:

--- mock_airflow.py	2024-08-09 10:54:55.000000000 -0400
+++ my_mock_airflow.py	2024-08-10 06:14:27.110493907 -0400
@@ -1,6 +1,6 @@
 import os
 import sys
-
+import time
 
 from func_timeout import func_timeout, FunctionTimedOut
 
@@ -27,12 +27,17 @@
 except FunctionTimedOut:
     print("FunctionTimedOut\n")
 
+time.sleep(2)
 
 print("After failed import redis")
 print(f"'redis' in sys.modules: {'redis' in sys.modules}")
 print(f"'redis.client' in sys.modules: {'redis.client' in sys.modules}")
 print()
 
+time.sleep(6)
+print("After waiting")
+print(f"'redis' in sys.modules: {'redis' in sys.modules}")
+print(f"'redis.client' in sys.modules: {'redis.client' in sys.modules}")
 
 print("Reimport mock_kombu_transport_redis")

Do that and run it with python3 -v -m mock_airflow, and what you’ll see is something like this:

$ python3 -v -m mock_airflow
import _frozen_importlib # frozen
import _imp # builtin
import '_thread' # <class '_frozen_importlib.BuiltinImporter'>
[...]
import 'asyncio.selector_events' # <_frozen_importlib_external.SourceFileLoader object at 0x7fb56ad6b350>
import 'asyncio.unix_events' # <_frozen_importlib_external.SourceFileLoader object at 0x7fb56ad3fc80>
import 'asyncio' # <_frozen_importlib_external.SourceFileLoader object at 0x7fb56bfeb7d0>
[...]
import 'redis.commands' # <_frozen_importlib_external.SourceFileLoader object at 0x7fb56a9af5f0>
# /tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__pycache__/lock.cpython-312.pyc matches /tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/lock.py
# code object from '/tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__pycache__/lock.cpython-312.pyc'
import 'redis.lock' # <_frozen_importlib_external.SourceFileLoader object at 0x7fb56aa84dd0>
import 'redis.client' # <_frozen_importlib_external.SourceFileLoader object at 0x7fb56a9ad790>
import 'redis.asyncio.client' # <_frozen_importlib_external.SourceFileLoader object at 0x7fb56bfe8f80>
[...]import 'redis.sentinel' # <_frozen_importlib_external.SourceFileLoader object at 0x7fb56bfe8bc0>
FunctionTimedOut

After failed import redis
'redis' in sys.modules: True
'redis.client' in sys.modules: True

# destroy mock_kombu_transport_redis
After waiting
'redis' in sys.modules: False
'redis.client' in sys.modules: True

In other words, sys.modules['redis'] only exists, on your initial check, because the attempt to import mock_kombu_transport_redis is still incomplete, waiting for the sleep(10) in redis/__init__.py to complete. When it does, the import tracing prints,

# destroy mock_kombu_transport_redis

…and both that module and redis are removed from sys.modules because they failed to successfully import. But a ton of the REST of Redis did successfully import, so now when you attempt to import redis again, those inner modules aren’t reinitialized because they’re already in sys.modules. And because they aren’t getting reimported, they don’t end up in the redis namespace.

You can easily create the same situation “by hand” with stock Redis (or any package). Try this in the REPL:

>>> import sys; from pprint import pp
>>> import redis
>>> redis.client
<module 'redis.client' from '/tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/client.py'>
>>> del redis
>>> sys.modules.pop('redis')
<module 'redis' from '/tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__init__.py'>
>>> import redis
>>> pp(dir(redis))
['AuthenticationError',
 'AuthenticationWrongNumberOfArgsError',
 'BlockingConnectionPool',
 'BusyLoadingError',
 'ChildDeadlockedError',
 'Connection',
 'ConnectionError',
 'ConnectionPool',
 'CredentialProvider',
 'DataError',
 'InvalidResponse',
 'OutOfMemoryError',
 'PubSubError',
 'ReadOnlyError',
 'Redis',
 'RedisCluster',
 'RedisError',
 'ResponseError',
 'SSLConnection',
 'Sentinel',
 'SentinelConnectionPool',
 'SentinelManagedConnection',
 'SentinelManagedSSLConnection',
 'StrictRedis',
 'TimeoutError',
 'UnixDomainSocketConnection',
 'UsernamePasswordCredentialProvider',
 'VERSION',
 'WatchError',
 '__all__',
 '__builtins__',
 '__cached__',
 '__doc__',
 '__file__',
 '__loader__',
 '__name__',
 '__package__',
 '__path__',
 '__spec__',
 '__version__',
 'asyncio',
 'default_backoff',
 'from_url',
 'int_or_str',
 'metadata',
 'os',
 'sys',
 'time']

(For fun, as long as 'redis.client' is in sys.modules but 'redis' isn’t, you can even do an import redis.client or from redis.client import Redis, either of which will work fine — before or after import redis — but neither one will cause redis.client to actually be defined. Unless the redis.client module gets forcibly ejected from sys.modules and reloaded, the symbol won’t be imported into redis.)

…If you also sys.modules.pop('redis.client'), then an import redis WILL reimport redis.client and client will be a member of redis, but redis will still be missing all of the other redis.* symbols that don’t get reinitialized.

>>> del redis
>>> sys.modules.pop('redis.client')
<module 'redis.client' from '/tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/client.py'>
>>> sys.modules.pop('redis')
<module 'redis' from '/tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__init__.py'>
>>> import redis
# /tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__pycache__/__init__.cpython-312.pyc matches /tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__init__.py
# code object from '/tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__pycache__/__init__.cpython-312.pyc'
# /tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__pycache__/client.cpython-312.pyc matches /tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/client.py
# code object from '/tmp/mcve/venv3.12/lib64/python3.12/site-packages/redis/__pycache__/client.cpython-312.pyc'
import 'redis.client' # <_frozen_importlib_external.SourceFileLoader object at 0x7faf0f695be0>
import 'redis' # <_frozen_importlib_external.SourceFileLoader object at 0x7faf0f695700>
>>> 'client' in dir(redis)
True
>>> [x in dir(redis) for x in ['sentinel', 'backoff', 'cluster', 'crc']]
[False, False, False, False]