Repository navigation
Commit b403c71
fix(worker): the await is right, the reason given for it was wrong
Review finding 1 (correctness of the JUSTIFICATION, not of the code) plus
finding 3. No behaviour change: comments, a docstring, an assert message, and
one added assertion.
e086450 said an unconditional await was needed because a rate() that stops
yielding "cannot be stopped". That is false on this host. Stop is
worker.terminate() on the page side — unconditional, needing nothing from the
worker thread — and trinket's worker-client.js says so in its opening comment.
A wrong reason attached to a right decision is a liability: the next reader
checks it, finds Stop works fine without the await, and deletes the await.
The true reason is narrower and worse. In a worker the host delivers browser
events by CALLING __trinket_vpython_dispatch, and that call only gets a turn
when the running coroutine gives one up. A rate() that returns without awaiting
therefore dispatches no events and flushes no updates: the scene FREEZES while
the program runs flat out. Stop surviving that is what makes it subtle rather
than mild — a frozen picture from a program that is still running reads to a
student as "vpython is broken", not as a hang, and Stop clearing it up is
consistent with both stories. Corrected in all three places it appeared here;
the report's copy goes with the trinket commit.
Finding 3: test_rate_subtracts_user_code_time_from_the_period asserted only an
upper bound, so "sleep zero whenever the body did any work" — a plausible
mis-implementation of exactly this fix, and a much worse one than the flat sleep
it replaced — would have passed it. It now also asserts elapsed > ideal * 0.7:
compensating for the body's cost must not become free-running.
Tests: 31 passed (unchanged count; the new bound rides on the existing case).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7y9rTA1r8r8EhEnPSQenR1 parent e086450 commit b403c71
2 files changed
Lines changed: 30 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
183 | 183 | | |
184 | 184 | | |
185 | 185 | | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
186 | 192 | | |
187 | 193 | | |
188 | 194 | | |
189 | 195 | | |
190 | 196 | | |
191 | | - | |
192 | | - | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
193 | 203 | | |
194 | 204 | | |
195 | 205 | | |
| |||
214 | 224 | | |
215 | 225 | | |
216 | 226 | | |
217 | | - | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
218 | 230 | | |
219 | 231 | | |
220 | 232 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
128 | 128 | | |
129 | 129 | | |
130 | 130 | | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
135 | 146 | | |
136 | 147 | | |
137 | 148 | | |
| |||
0 commit comments