Fix empty kp hang#2799
Open
schanzer wants to merge 4 commits into
Open
Conversation
The s-expression reader's inner loops (sread_line, sread_block_comment, sread_atom, sread_string) only broke on specific characters and never checked for nil, which io.read(1) returns at EOF. On an empty or truncated input the reader looped forever at 100% CPU. sread() also dispatched a nil peek into sread_atom, so even a 0-byte file would spin. This was the root of the intermittent `make` hang on "Make pathway-tocs.js": a 0-byte .workbook-lessons.rkt.kp fed sread_file, which never returned. Treat nil (EOF) as terminating in every loop, and have sread() return false when the input is exhausted. Verified: a 0-byte file now returns false immediately, a valid list still parses identically, and a truncated (unterminated) list returns without hanging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The loop guarded only on file_exists_p, then fed the file to sread_file and iterated the result with ipairs. A 0-byte .workbook-lessons.rkt.kp slipped past the existence check; with the sread EOF fix it now returns false, which ipairs() can't iterate. Guard on the parsed result: if sread_file doesn't return a table, warn and skip the course. Verified by emptying __sample's .kp and running the script -- it now skips __sample with a warning and emits a well-formed pathway-tocs.js for the remaining courses, instead of hanging or crashing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
make-workbook-lessons-list.lua opens its output with 'w', truncating the target to 0 bytes the instant it starts; a crash/interrupt then leaves a 0-byte file behind, which a later incremental build's reader consumed (the source of the pathway-tocs hang). The no-lesson-order branch also created a deliberately empty file via `touch`. Publish atomically: write to a .tmp and mv it into place only on success, so an interrupted run leaves the previous good file intact rather than an empty one. For the no-lesson-order case, write a valid empty list "()" instead of a 0-byte touch. Verified: bash -n passes, "()" parses as an empty list, and a simulated mid-populate failure leaves the prior file contents untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
|
As mentioned in private slack, an alternate fix -- a simple rewrite of |
Member
Author
|
@ds26gte I appreciate the poke! I'm testing your fix locally today on my local machine, and if the problem doesn't re-appear I'll merge |
…eing interruptible
Member
Author
|
@ds26gte Confirming that this fixes one issue, but not all of them. Your fix eliminates one of the sources of a 0-byte files (pathways with no lesson-order.txt files), but not all of them (a halted I've cherry-picked your change into this branch, and will merge it to master after some testing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The actual symptom of the stale cache bug:
massage-coursecould leave an empty cache filesreadcalls in Phase 2 try to parse itThe three commits in this PR adds a
nilcheck to thesreadcalls, make sure thatmerge-pathway-tocscan throw an error and keep going rather than choking, and makesmassage-courseatomic so that it never generates an empty file