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

Popen used in a way that may cause deadlock #198

Open
ekoyle opened this issue Jul 30, 2024 · 0 comments
Open

Popen used in a way that may cause deadlock #198

ekoyle opened this issue Jul 30, 2024 · 0 comments

Comments

@ekoyle
Copy link

ekoyle commented Jul 30, 2024

I noticed a minor issue here:

build = subprocess.Popen(
build_command,
shell=True,
cwd=clone.working_dir,
env=env,
bufsize=bufsize,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
try:
buildlog = self.package_build_log(clone.working_dir)
with open(buildlog, "wb") as f:
LOG.info(
'installing "%s": writing build log: %s',
package,
buildlog,
)
f.write("=== STDERR ===\n".encode(std_encoding(sys.stderr)))
while True:
data = build.stderr.read(bufsize)
if data:
f.write(data)
else:
break
f.write("=== STDOUT ===\n".encode(std_encoding(sys.stdout)))
while True:
data = build.stdout.read(bufsize)
if data:
f.write(data)
else:
break

This pattern will cause a deadlock if the child process writes enough to stdout to fill the buffer and block.

p = subprocess.Popen(..., stdout=subrpocess.PIPE, stderr=subprocess.PIPE)

# read p.sdterr until EOF
while True:
    # this read() may cause a deadlock as the stdout pipe could fill, causing the child process
    # to block forever on a write to stdout while the parent process is blocking on this read
    # and therefore not reading from p.stdout
    data = p.stderr.read(bufsize)

    if data:
        f.write(data)
    else:
        break

# read p.stdout until EOF
...

It is recommended to use Popen.communicate() to safely handle cases where subprocess.PIPE is used in a Popen (especially more than once) to avoid this issue. If you must read directly from more than one pipe, then epoll(), select(), or similar should be used to avoid blocking reads which could result in a deadlock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants