Skip to content

Fix a post-request crasher when we try to do sandbox reuse#22

Merged
erikrose merged 4 commits into
mainfrom
sandbox-reuse-crash
Dec 2, 2025
Merged

Fix a post-request crasher when we try to do sandbox reuse#22
erikrose merged 4 commits into
mainfrom
sandbox-reuse-crash

Conversation

@erikrose
Copy link
Copy Markdown
Member

@erikrose erikrose commented Nov 26, 2025

Deal with the case when await_request() returns None, indicating "there are no more requests this session". It's an optional in the WIT, though I never saw this crash on my local machine when I first wrote the code. Perhaps something changed in Viceroy. The crash happens after the request is served, so CI didn't catch it before. Now it does, though it takes several secs to show up, which makes me very sad. But a slow, sleepy CI is better than an uncaught crasher.

Also add more-ordinary tests for Game Of Life.

With compression, a board size of 45 no longer crashes. Move the talk of limits to the place where we
@erikrose erikrose requested a review from posborne November 26, 2025 17:41
@erikrose erikrose force-pushed the sandbox-reuse-crash branch from 9458106 to 6dce28f Compare November 26, 2025 18:11
…indicating "there are no more requests this session". It's an `optional` in the WIT, though it never returned `None` before. I suspect multi-request is never happening anymore. Perhaps something changed in Viceroy, but this crash happens in Viceroy's `erik/python-sdk-compatible-2` and `main` (80c9e1486e5bbe559ec0194714bb668988984993). The crash happens *after* the request is served, so CI didn't catch it. It does now.

Also add tests for Game Of Life.
Comment thread tests/test_game_of_life_example.py Outdated

# The error about failed sandbox reuse comes *after* the request has
# succeeded. And it seems to take forever to show up.
sleep(4) # 2 is not enough. I am sad.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you know why this is? Is this a latent gc collection or something else going on within viceroy's implementation?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I really wish I knew. I didn't chase it to exhaustion. It manifested as a Heisenbug wherein my REPL explorations in the debugger (which generally took >4s) would turn up the message but the tests would not.

And now, futzing with it some more, it passes without the sleep. Let's see what CI does…

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nm; I was testing it wrong—I forgot to comment out my bug fix. Anyway, I did find something interesting and make a small improvement: today, it needs only a .5s sleep.

@erikrose erikrose force-pushed the sandbox-reuse-crash branch from 4b12dbc to b1e49f2 Compare December 1, 2025 15:58
Today, it seems not to need as long. The unpredictability displeases me.
@erikrose erikrose merged commit 202c8ba into main Dec 2, 2025
3 checks passed
@erikrose erikrose deleted the sandbox-reuse-crash branch February 3, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants