8000 gh-96522: Fix deadlock in pty.spawn (#96639) · python/cpython@9c5aa89 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9c5aa89

Browse files
zhangyoufuzhangyoufu
authored
gh-96522: Fix deadlock in pty.spawn (#96639)
1 parent c26d03d commit 9c5aa89

File tree

4 files changed

+56
-22
lines changed

4 files changed

+56
-22
lines changed

‎Lib/pty.py

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,6 @@ def fork():
115115
# Parent and child process.
116116
return pid, master_fd
117117

118-
def _writen(fd, data):
119-
"""Write all the data to a descriptor."""
120-
while data:
121-
n = os.write(fd, data)
122-
data = data[n:]
123-
124118
def _read(fd):
125119
"""Default read function."""
126120
return os.read(fd, 1024)
@@ -130,9 +124,42 @@ def _copy(master_fd, master_read=_read, stdin_read=_read):
130124
Copies
131125
pty master -> standard output (master_read)
132126
standard input -> pty master (stdin_read)"""
133-
fds = [master_fd, STDIN_FILENO]
134-
while fds:
135-
rfds, _wfds, _xfds = select(fds, [], [])
127+
if os.get_blocking(master_fd):
128+
# If we write more than tty/ndisc is willing to buffer, we may block
129+
# indefinitely. So we set master_fd to non-blocking temporarily during
130+
# the copy operation.
131+
os.set_blocking(master_fd, False)
132+
try:
133+
_copy(master_fd, master_read=master_read, stdin_read=stdin_read)
134+
finally:
135+
# restore blocking mode for backwards compatibility
136+
os.set_blocking(master_fd, True)
137+
return
138+
high_waterlevel = 4096
139+
stdin_avail = master_fd != STDIN_FILENO
140+
stdout_avail = master_fd != STDOUT_FILENO
141+
i_buf = b''
142+
o_buf = b''
143+
while 1:
144+
rfds = []
145+
wfds = []
146+
if stdin_avail and len(i_buf) < high_waterlevel:
147+
rfds.append(STDIN_FILENO)
148+
if stdout_avail and len(o_buf) < high_waterlevel:
149+
rfds.append(master_fd)
150+
if stdout_avail and len(o_buf) > 0:
151+
wfds.append(STDOUT_FILENO)
152+
if len(i_buf) > 0:
153+
wfds.append(master_fd)
154+
155+
rfds, wfds, _xfds = select(rfds, wfds, [])
156+
157+
if STDOUT_FILENO in wfds:
158+
try:
159+
n = os.write(STDOUT_FILENO, o_buf)
160+
o_buf = o_buf[n:]
161+
except OSError:
162+
stdout_avail = False
136163

137164
if master_fd in rfds:
138165
# Some OSes signal EOF by returning an empty byte string,
@@ -144,15 +171,18 @@ def _copy(master_fd, master_read=_read, stdin_read=_read):
144171
if not data: # Reached EOF.
145172
return # Assume the child process has exited and is
146173
# unreachable, so we clean up.
147-
else:
148-
os.write(STDOUT_FILENO, data)
174+
o_buf += data
175+
176+
if master_fd in wfds:
177+
n = os.write(master_fd, i_buf)
178+
i_buf = i_buf[n:]
149179

150-
if STDIN_FILENO in rfds:
180+
if stdin_avail and STDIN_FILENO in rfds:
151181
data = stdin_read(STDIN_FILENO)
152182
if not data:
153-
fds.remove(STDIN_FILENO)
183+
stdin_avail = False
154184
else:
155-
_writen(master_fd, data)
185+
i_buf += data
156186

157187
def spawn(argv, master_read=_read, stdin_read=_read):
158188
"""Create a spawned process."""

‎Lib/test/test_pty.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,8 @@ def setUp(self):
312312
self.orig_pty_waitpid = pty.waitpid
313313
self.fds = [] # A list of file descriptors to close.
314314
self.files = []
315-
self.select_rfds_lengths = []
316-
self.select_rfds_results = []
315+
self.select_input = []
316+
self.select_output = []
317317
self.tcsetattr_mode_setting = None
318318

319319
def tearDown(self):
@@ -350,8 +350,8 @@ def _socketpair(self):
350350

351351
def _mock_select(self, rfds, wfds, xfds):
352352
# This will raise IndexError when no more expected calls exist.
353-
self.assertEqual(self.select_rfds_lengths.pop(0), len(rfds))
354-
return self.select_rfds_results.pop(0), [], []
353+
self.assertEqual((rfds, wfds, xfds), self.select_input.pop(0))
354+
return self.select_output.pop(0)
355355

356356
def _make_mock_fork(self, pid):
357357
def mock_fork():
@@ -374,11 +374,13 @@ def test__copy_to_each(self):
374374
os.write(masters[1], b'from master')
375375
os.write(write_to_stdin_fd, b'from stdin')
376376

377-
# Expect two select calls, the last one will cause IndexError
377+
# Expect three select calls, the last one will cause IndexError
378378
pty.select = self._mock_select
379-
self.select_rfds_lengths.append(2)
380-
self.select_rfds_results.append([mock_stdin_fd, masters[0]])
381-
self.select_rfds_lengths.append(2)
379+
self.select_input.append(([mock_stdin_fd, masters[0]], [], []))
380+
self.select_output.append(([mock_stdin_fd, masters[0]], [], []))
381+
self.select_input.append(([mock_stdin_fd, masters[0]], [mock_stdout_fd, masters[0]], []))
382+
self.select_output.append(([], [mock_stdout_fd, masters[0]], []))
383+
self.select_input.append(([mock_stdin_fd, masters[0]], [], []))
382384

383385
with self.assertRaises(IndexError):
384386
pty._copy(masters[0])

‎Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2060,6 +2060,7 @@ Yuxiao Zeng
20602060
Uwe Zessin
20612061
Cheng Zhang
20622062
George Zhang
2063+
Youfu Zhang
20632064
Charlie Zhao
20642065
Kai Zhu
20652066
Tarek Ziadé

‎Misc/NEWS.d/next/Library/2022-09-07-09-32-07.gh-issue-96522.t73oqp.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix potential deadlock in pty.spawn()

0 commit comments

Comments
 (0)
0