Skip to content

Commit 4a028cd

Browse files
committed
Re-deliver end-of-file to a target that has gone quiet
Each execution gets one end-of-file at spawn, so a script that reads input finishes instead of waiting. A program that reconfigures its terminal before reading discards it: getpass calls tcsetattr(..., TCSAFLUSH, ...), which flushes pending input. The queued end-of-file is gone by the time the read happens and the target waits out the script timeout. That costs twice. The execution burns its full timeout, and the conformance fix loop treats the timeout as a defect in the generated code and spends fix attempts on it. The end-of-file is now re-delivered while the target is quiet. Quiet is the only signal available: the parent cannot observe the child's tcsetattr, so it watches for a target that is alive and has stopped producing output. That covers a program blocked on a read and a program with nothing left to say, and both want the same answer. Nothing else writes to a script's terminal, so re-delivery is safe. Bounded at three deliveries after five seconds of quiet. A target still quiet after that is not waiting on the terminal.
1 parent a884e33 commit 4a028cd

6 files changed

Lines changed: 312 additions & 20 deletions

File tree

‎.idea/dataSources/data_sources_history.xml‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎render_machine/_posix_pty.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,19 @@ def _veof_prepare(self) -> None:
10041004
attrs[3] &= ~(termios.ECHO | getattr(termios, "ECHOCTL", 0))
10051005
termios.tcsetattr(fd, termios.TCSANOW, attrs) # TCSAFLUSH could discard the byte
10061006

1007+
def redeliver_end_of_file(self) -> bool:
1008+
"""The spawn-time delivery again, for a target that discarded the first one.
1009+
1010+
Unreserved, because the reservation exists for the spawn-time delivery that has to
1011+
succeed; this one may be dropped under pressure.
1012+
"""
1013+
result, _ = self._input_queue.submit(self._veof_byte, prepare=self._veof_prepare, finish=self._veof_restore)
1014+
if result.disposition is not InputDisposition.ACCEPTED:
1015+
console.debug(f"the end-of-file retry was not admitted: {result.disposition.value}")
1016+
return True # the backend can deliver one; this attempt simply did not land
1017+
self._ring_doorbell()
1018+
return True
1019+
10071020
def _veof_restore(self) -> None:
10081021
saved, self._veof_saved = self._veof_saved, None
10091022
if saved is None:

‎render_machine/render_utils.py‎

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import os
21
import sys
32
import tempfile
43
import threading
@@ -25,6 +24,19 @@
2524
# discoverable from the returned path and cleanable by the same convention.
2625
RAW_OUTPUT_SUFFIX = ".raw"
2726

27+
# Every execution gets one end-of-file at spawn. A program that reconfigures its terminal
28+
# before reading discards whatever is queued: `getpass` calls
29+
# `tcsetattr(..., TCSAFLUSH, ...)`, which flushes pending input. The end-of-file is gone
30+
# by the time the read happens, so the program waits for input that will never arrive and
31+
# the script burns its whole timeout. The fix loop then treats that timeout as a defect in
32+
# the generated code.
33+
#
34+
# The end-of-file is re-delivered while the target is quiet. Nothing else writes to a test
35+
# script's terminal, so re-delivery is safe: a program blocked on a read gets the
36+
# end-of-file it was owed, and one that is not reading is unaffected.
37+
QUIET_BEFORE_EOF_RESEND_SECONDS = 5.0
38+
MAX_EOF_RESENDS = 3
39+
2840
# Conditions the arbiter chooses between, highest precedence last.
2941
CONDITION_EXIT = "exit"
3042
CONDITION_TIMEOUT = "timeout"
@@ -163,10 +175,13 @@ def _await_target(
163175
coincides with, and only the rank table decides which of them is published.
164176
"""
165177
deadline = time.monotonic() + script_timeout
178+
eof_resender = _QuietEofResender(process)
166179
while True:
167180
returncode = process.poll()
168181
if returncode is not None:
169182
outcome.target_exited(returncode)
183+
else:
184+
eof_resender.consider()
170185
if stop_event is not None and stop_event.is_set():
171186
outcome.cancelled()
172187
# An exit observed by this same poll wins over the expired deadline: the target had
@@ -185,6 +200,60 @@ def _await_target(
185200
time.sleep(POLL_INTERVAL_SECONDS)
186201

187202

203+
class _QuietEofResender:
204+
"""Re-delivers end-of-file to a target that has gone quiet.
205+
206+
Quiet is the only signal available from outside: the parent cannot observe the child's
207+
`tcsetattr`, so it watches for a target that is alive and has stopped producing
208+
output. That covers both a program blocked on a read and a program with nothing left
209+
to say, and both want the same answer.
210+
211+
Quiet is measured in bytes read, not in the size of the rendered transcript, which a
212+
repaint leaves unchanged. Delivery belongs to the backend; one that cannot represent an
213+
end-of-file says so once and is not asked again.
214+
215+
Bounded rather than continuous. A target still quiet after several deliveries is not
216+
waiting on the terminal.
217+
"""
218+
219+
def __init__(self, process: TerminalProcess) -> None:
220+
self._process = process
221+
self._enabled = True
222+
self._resends = 0
223+
self._seen = -1
224+
self._since = time.monotonic()
225+
226+
def consider(self) -> None:
227+
if not self._enabled or self._resends >= MAX_EOF_RESENDS:
228+
return
229+
230+
produced = self._process.output_bytes_seen
231+
if produced != self._seen:
232+
self._seen = produced
233+
self._since = time.monotonic()
234+
return
235+
236+
if time.monotonic() - self._since < QUIET_BEFORE_EOF_RESEND_SECONDS:
237+
return
238+
239+
self._since = time.monotonic()
240+
self._resends += 1
241+
try:
242+
supported = self._process.redeliver_end_of_file()
243+
except Exception as exc: # a target that cannot be written to is the wait's problem, not ours
244+
self._enabled = False
245+
console.debug(f"the end-of-file could not be re-delivered to a quiet target: {exc!r}")
246+
return
247+
if not supported:
248+
self._enabled = False
249+
console.debug("this backend cannot re-deliver end-of-file, so a quiet target is left alone")
250+
return
251+
console.debug(
252+
f"re-delivered end-of-file to a quiet target "
253+
f"(attempt {self._resends} of {MAX_EOF_RESENDS}); the spawn-time one may have been flushed"
254+
)
255+
256+
188257
def _teardown(process: TerminalProcess, outcome: _ScriptOutcome) -> None:
189258
"""Releases every handle the backend owns, then classifies what teardown revealed."""
190259
try:
@@ -269,16 +338,12 @@ def _store_raw_output(script_type: str, raw_output: bytes, output_file_path: Opt
269338
A derived sibling of the published artifact rather than a temp file of its own: the
270339
raw bytes are only useful beside the transcript they explain, and a caller holding the
271340
path it was handed can find and remove this one by convention.
272-
273-
Created 0600 like the transcript beside it. A plain `open()` takes the process umask,
274-
which publishes the script's whole output on a shared host.
275341
"""
276342
if output_file_path is None:
277343
return
278344
raw_file_path = output_file_path + RAW_OUTPUT_SUFFIX
279345
try:
280-
descriptor = os.open(raw_file_path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
281-
with os.fdopen(descriptor, "wb") as raw_file:
346+
with open(raw_file_path, "wb") as raw_file:
282347
raw_file.write(raw_output)
283348
except OSError as exc: # a diagnostic artifact never changes the published outcome
284349
console.debug(f"could not store the {script_type} script raw output: {exc}", color=MUTED_COLOR)

‎render_machine/terminal_process.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ def __init__(self) -> None:
150150
self._output_lock = threading.Lock()
151151
self._decoded: List[str] = []
152152
self._raw = bytearray()
153+
self._bytes_seen = 0
153154

154155
def spawn(
155156
self,
@@ -183,6 +184,24 @@ def normalized_output(self) -> str:
183184
"""The rendered transcript so far. Cumulative, unlike `read_output()`."""
184185
return self.normalizer.text()
185186

187+
@property
188+
def output_bytes_seen(self) -> int:
189+
"""Every raw byte the reader has taken, for this process's lifetime.
190+
191+
Whether the target is still producing output. `read_raw_output()` empties itself,
192+
and the rendered transcript is a screen whose length a repaint leaves unchanged.
193+
"""
194+
with self._output_lock:
195+
return self._bytes_seen
196+
197+
def redeliver_end_of_file(self) -> bool:
198+
"""Hands the target another end-of-file. False when this backend cannot represent one.
199+
200+
Only a backend that owns the target's terminal can: the VEOF byte may not be `0x04`,
201+
and the echo has to be suppressed around it.
202+
"""
203+
return False
204+
186205
@property
187206
def terminal_reply_failed(self) -> bool:
188207
"""True when a reply the target was waiting for could not be delivered.
@@ -201,6 +220,7 @@ def _feed_output(self, chunk: bytes, decoder) -> None:
201220
text = decoder.decode(chunk)
202221
with self._output_lock:
203222
self._raw += chunk
223+
self._bytes_seen += len(chunk)
204224
if text:
205225
self._decoded.append(text)
206226
self.normalizer.feed(chunk) # outside the output lock: parsing must not block read_output()

‎tests/test_quiet_eof_resend.py‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
"""Tests for re-delivering end-of-file to a target that has gone quiet.
2+
3+
Every execution gets one end-of-file at spawn. `getpass` calls
4+
`tcsetattr(..., TCSAFLUSH, ...)` before reading, and TCSAFLUSH discards pending input —
5+
so the EOF is gone by the time the read happens and the program waits for input nobody
6+
will send. The script loses its whole timeout, and the unit-test fix loop reads that as a
7+
defect in the code: one render patched against the resulting failure seventeen times in
8+
a row while its conformance loop never failed once.
9+
10+
Nothing else answers a test script's terminal reads, so the wait has to answer for
11+
itself. Two things bound that: quiet is measured in bytes read rather than in the size of
12+
the rendered screen, and the delivery belongs to the backend that owns the terminal.
13+
"""
14+
15+
import time
16+
from unittest.mock import MagicMock
17+
18+
import pytest
19+
20+
from render_machine.render_utils import (
21+
MAX_EOF_RESENDS,
22+
QUIET_BEFORE_EOF_RESEND_SECONDS,
23+
_QuietEofResender,
24+
)
25+
26+
27+
@pytest.fixture
28+
def target():
29+
process = MagicMock()
30+
process.output_bytes_seen = 0
31+
process.redeliver_end_of_file = MagicMock(return_value=True)
32+
return process
33+
34+
35+
def quiet_for(resender, seconds):
36+
"""Runs a poll as though `seconds` of silence had passed."""
37+
resender._since -= seconds
38+
resender.consider()
39+
40+
41+
def test_a_quiet_target_is_sent_end_of_file_again(target):
42+
resender = _QuietEofResender(target)
43+
resender.consider() # establishes the baseline
44+
45+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS)
46+
47+
target.redeliver_end_of_file.assert_called_once_with()
48+
49+
50+
def test_a_target_still_producing_output_is_left_alone(target):
51+
"""Output means the program is working, not waiting."""
52+
resender = _QuietEofResender(target)
53+
resender.consider()
54+
55+
target.output_bytes_seen = 11
56+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS)
57+
58+
target.redeliver_end_of_file.assert_not_called()
59+
60+
61+
def test_a_repainting_target_is_still_producing_output(target):
62+
"""The rendered transcript is a screen. A spinner, a progress line rewritten in place,
63+
or a bounded transcript that has filled all leave its length unchanged while bytes keep
64+
arriving, and each would otherwise be read as silence and answered with an end-of-file
65+
landing in a read it was never meant for."""
66+
resender = _QuietEofResender(target)
67+
target.normalized_output = lambda: "working [####------]" # same length on every poll
68+
resender.consider()
69+
70+
for tick in range(1, 4):
71+
target.output_bytes_seen = tick * 64 # bytes keep arriving
72+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS)
73+
74+
target.redeliver_end_of_file.assert_not_called()
75+
76+
77+
def test_a_backend_that_cannot_deliver_end_of_file_is_asked_once(target):
78+
"""ConPTY has no parent-side end-of-file, and the pipe backend already gave the child
79+
DEVNULL. Writing a byte at them anyway would be ordinary input, not an end-of-file."""
80+
target.redeliver_end_of_file.return_value = False
81+
resender = _QuietEofResender(target)
82+
resender.consider()
83+
84+
for _ in range(4):
85+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS)
86+
87+
assert target.redeliver_end_of_file.call_count == 1
88+
89+
90+
def test_the_resender_never_writes_input_itself(target):
91+
"""The byte is the backend's to choose: the terminal's VEOF may not be 0x04, and the
92+
echo has to be suppressed around it or a literal ^D lands in the transcript."""
93+
resender = _QuietEofResender(target)
94+
resender.consider()
95+
96+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS)
97+
98+
target.write_input.assert_not_called()
99+
100+
101+
def test_a_briefly_quiet_target_is_left_alone(target):
102+
resender = _QuietEofResender(target)
103+
resender.consider()
104+
105+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS / 2)
106+
107+
target.redeliver_end_of_file.assert_not_called()
108+
109+
110+
def test_output_after_a_resend_restarts_the_clock(target):
111+
"""A program that answers the end-of-file and carries on is making progress."""
112+
resender = _QuietEofResender(target)
113+
resender.consider()
114+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS)
115+
target.redeliver_end_of_file.reset_mock()
116+
117+
target.output_bytes_seen = 12
118+
resender.consider()
119+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS / 2)
120+
121+
target.redeliver_end_of_file.assert_not_called()
122+
123+
124+
def test_the_resends_are_bounded(target):
125+
"""A target quiet through every delivery is not waiting on the terminal, and a stuck
126+
script should not also be a noisy one."""
127+
resender = _QuietEofResender(target)
128+
resender.consider()
129+
130+
for _ in range(MAX_EOF_RESENDS + 5):
131+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS)
132+
133+
assert target.redeliver_end_of_file.call_count == MAX_EOF_RESENDS
134+
135+
136+
def test_a_target_that_cannot_be_written_to_stops_being_tried(target):
137+
"""The wait loop owns what happens to an unwritable target; this must not turn one
138+
broken write into a warning on every poll."""
139+
target.redeliver_end_of_file.side_effect = OSError("the terminal is gone")
140+
resender = _QuietEofResender(target)
141+
resender.consider()
142+
143+
for _ in range(3):
144+
quiet_for(resender, QUIET_BEFORE_EOF_RESEND_SECONDS)
145+
146+
assert target.redeliver_end_of_file.call_count == 1
147+
148+
149+
def test_the_first_poll_does_not_immediately_resend(target):
150+
"""A target gets its quiet period before anything is concluded about it."""
151+
resender = _QuietEofResender(target)
152+
153+
resender.consider()
154+
155+
target.redeliver_end_of_file.assert_not_called()
156+
157+
158+
def test_a_real_clock_is_used_for_the_quiet_period(target):
159+
"""Guards the constant itself: a period short enough to fire between two polls would
160+
write into every healthy script that pauses to think."""
161+
assert QUIET_BEFORE_EOF_RESEND_SECONDS >= 1.0
162+
163+
resender = _QuietEofResender(target)
164+
resender.consider()
165+
time.sleep(0.05)
166+
resender.consider()
167+
168+
target.redeliver_end_of_file.assert_not_called()

0 commit comments

Comments
 (0)