Skip to content
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

Can not communicate with subprocess during interpreter shutdown on windows #115219

Closed
tacaswell opened this issue Feb 9, 2024 · 4 comments
Closed
Assignees
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes OS-windows type-bug An unexpected behavior, bug, or error

Comments

@tacaswell
Copy link
Contributor

tacaswell commented Feb 9, 2024

Bug report

Bug description:

In #104826 (backported to 3.12 in #105277) to fix #104690 disallowed creating new threads during interpreter finalization. This poses a problem on windows because the Popen._communicate uses a thread (see matplotlib/matplotlib#27437 (comment) for discussion).

In Matplotlib we are holding onto a latex process and triggering this via a weakref finalize, but a minimal reproducer is:

from subprocess import Popen, PIPE
import atexit

# anything that holds stdin and stdout should work
proc = Popen(['powershell'], stdin=PIPE, stdout=PIPE)

def cleanup():
    print('hi bob')
    proc.kill()
    proc.communicate()

atexit.register(cleanup)

xref matplotlib/matplotlib#27437

CPython versions tested on:

3.12

Operating systems tested on:

Windows

@tacaswell tacaswell added the type-bug An unexpected behavior, bug, or error label Feb 9, 2024
@serhiy-storchaka serhiy-storchaka added 3.12 bugs and security fixes 3.13 bugs and security fixes labels Feb 9, 2024
@serhiy-storchaka
Copy link
Member

cc @chgnrdv, @gpshead

@zooba
Copy link
Member

zooba commented Feb 12, 2024

(cc @ericsnowcurrently)

One possibility is to transform the Windows code here into a purely native thread, rather than a Python thread, but that is likely to break people who are passing file objects to stdout=/stderr= besides our constants. That's probably unlikely, but we also shouldn't break it.

What might be better, and why I pinged Eric, is to find a finalization ordering where we can run atexit callbacks and then require all threads to be exited. That may not help with the similar GC issues, but it seems like atexit ought to be okay with running a bit earlier.

@ericsnowcurrently
Copy link
Member

See gh-116677.

@gpshead
Copy link
Member

gpshead commented Mar 19, 2024

This subprocess on Windows atexit or from a non-daemon thread use case is a good one. I'm going to mark this a duplicate of our more general issue #113964 on the topic where we've got a PR and likely a second one in the works to address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

7 participants