Skip to content

Fix gpg command hanging #66

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.

Sign up for GitHub

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

Merged
doomedraven merged 1 commit into from
Jan 8, 2026
+24 −9
Merged

Fix gpg command hanging #66

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions sflock/unpack/pgp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,30 @@ def unpack(self, password: str = None, duplicates=None):
# ToDo
# locked system call occurred during sandboxing!\nip=0x7f9d2bc0fa97 sp=0x7ffdcb8d5eb8 abi=0 nr=102 syscall=getuid
# ret = self.zipjail(filepath, dirpath, "-o", os.path.join(dirpath, "extracted"), "--passphrase=%s" % (password or ""), filepath)
p = subprocess.Popen(
(self.exe, "--decrypt", "--batch", "-o", os.path.join(dirpath, "extracted"), "--passphrase=%s" % (password or ""), filepath),
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)

return_code = p.wait()
_, _ = p.communicate()
p = None
try:
p = subprocess.Popen(
(self.exe, "--decrypt", "--batch", "-o", os.path.join(dirpath, "extracted"),
"--passphrase=%s" % (password or ""), filepath),
stdin=subprocess.DEVNULL,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)

stdout, stderr = p.communicate(timeout=30)
Copy link

@gemini-code-assist gemini-code-assist bot Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The stdout and stderr variables are captured from p.communicate() but are not used later in the function. To make it clear that they are intentionally ignored, it's conventional to name them with an underscore (_).

Suggested change
stdout, stderr = p.communicate(timeout=30)
_, _ = p.communicate(timeout=30)

return_code = p.returncode

except subprocess.TimeoutExpired:
if p:
p.kill()
_, _ = p.communicate()
return_code = 1
except Exception:
if p:
p.kill()
p.wait()
Copy link

@gemini-code-assist gemini-code-assist bot Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using p.wait() here can be risky. It's safer to use p.communicate() to ensure the subprocess's output pipes are drained, preventing potential deadlocks. This would also make the error handling consistent with the TimeoutExpired block above.

Suggested change
p.wait()
_, _ = p.communicate()

Copy link
Author

@itamarga itamarga Jan 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better not to call communicate() in a generic except block. If the error was caused by the pipe itself being broken (e.g., BrokenPipeError or OSError), calling communicate() might raise another exception.
wait() is considered safe after kill()

return_code = 1

ret = not return_code
if not ret:
return []
Expand Down