New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Logging displays wrong "processName" if "sys.modules" is cleared in child process #82943
Comments
Hi. In order to display the process name in logs, the "logging" module checks for the presence of "multiprocessing" in "sys.modules" before calling "current_process()". If "multiprocessing" is not found in "sys.modules", it assumes that the current process is the main one. See : cpython/Lib/logging/__init__.py Lines 340 to 341 in af46450
However, nothing prevents a child process to delete "sys.module['multiprocessing']", which causes the process name to be wrongly displayed as "MainProcess". I attached a reproducible example, but this is straightforward to understand. Although it should not happen very often in practice, it is still theoretically possible. Obviously, one could say "just don't clear sys.modules", but I suppose there might exist tools doing such thing for good reasons (like resetting the test environment). Issues which lead to the current implementation: Possible fixes:
|
This will be a rare use case, and I would expect anyone clearing sys.modules in the child process to handle this consequence themselves (e.g. by reimporting multiprocessing after clearing out the other modules, since they apparently still want some of multiprocessing's functionality in the child process). I'll certainly look at PRs to address the issue using an import of multiprocessing if it's not already there. I'm not so keen on the other approaches you've suggested. |
See also bpo-8200, which relates to the way the code is currently. |
So this will make logging depending on the multiprocess module? Even if you do not use multiprocessing it will be imported in any case when you use logging. |
As the PR currently is, it will, unless you turn this off by setting logging.logMultiprocessing=False. The other alternative we are discussing is: revert back to not importing multiprocessing if it's not there. If it's there we use it, but if it's not we set processName to something like f"pid={os.getpid()}" instead of "MainThread". (1) os is already imported in logging. (2) In most cases where multiprocessing is not imported it's because there is only one process, so processName will not likely be used as all. (3) In the edge case of shutdown or someone clearing sys.modules the processName will not be pretty, but it will at least be correct. |
I just pushed a second PR with the alternative solution - it calculates the name on a best-effort basis, and doesn't import anything that's not already there. |
We've decided for now to leave the code's behavior as it is (the alternatives are worse) and to keep only the test improvements. See discussion on PR 22142. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: