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

Fix Channel.close() race condition in pipe.py #2274

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
34 changes: 30 additions & 4 deletions paramiko/pipe.py
Expand Up @@ -25,6 +25,7 @@
will trigger as readable in `select <select.select>`.
"""

import errno
import sys
import os
import socket
Expand All @@ -46,25 +47,50 @@ def __init__(self):
self._closed = False

def close(self):
os.close(self._rfd)
os.close(self._wfd)
# used for unit tests:
self._closed = True

# We cannot do anything about closing errors. It is only a
# "best effort" approach
try:
os.close(self._rfd)
except:
pettitpeon marked this conversation as resolved.
Show resolved Hide resolved
pass
try:
os.close(self._wfd)
except:
pettitpeon marked this conversation as resolved.
Show resolved Hide resolved
pass

def fileno(self):
return self._rfd

def clear(self):
if not self._set or self._forever:
return

# .read() does not need to handle a race condition with .close()
# because the pipe is created, cleared and closed from the same
# "client thread". The "server thread" sets the pipe and only .set()
# suffers from a race condition with .close()
os.read(self._rfd, 1)
self._set = False

def set(self):
if self._set or self._closed:
return
self._set = True
os.write(self._wfd, b"*")

# This try fixes a race condition with .close()
# 1. The write/server thread sees ._closed == False and continues
# 2. The close/client thread closes the descriptors before the
# write/server thread writes.
# 3. The write/server fails to write() because the FD has been closed
try:
os.write(self._wfd, b"*")
except OSError as e:
if e.errno == errno.EBADF and self._closed:
# The pipe was closed, no need to do anything
return
raise e

def set_forever(self):
self._forever = True
Expand Down