fix: prevent sidecar crash when apply_patch deletes a large/binary file#27658
Open
wangzexi wants to merge 1 commit into
Open
fix: prevent sidecar crash when apply_patch deletes a large/binary file#27658wangzexi wants to merge 1 commit into
wangzexi wants to merge 1 commit into
Conversation
When deleting a file via apply_patch, the entire file content was passed to createTwoFilesPatch and the resulting diff stored verbatim in SQLite. For a 142 MB binary file this produces a ~380 MB string; Node's sqlite module then passes it to v8::String::NewFromUtf8 which hits V8's internal string-length assertion and SIGTRAPs the sidecar process. Guard both the delete path in apply_patch.ts (skip diff generation when content > 512 KB) and trimDiff in edit.ts (early-return empty string when the diff itself already exceeds 512 KB). Closes anomalyco#27657
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
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.
Issue for this PR
Closes #27657
Type of change
What does this PR do?
When
apply_patchhandles adeleteoperation, it callscreateTwoFilesPatch(filePath, filePath, entireContent, "")and stores the result in the SQLitepart.datacolumn. For a binary or large file (e.g. a 142 MB.dmg) this produces a diff string of ~380 MB. The next time any query runsStatementSync.all()over that session's parts, Node's sqlite module passes that 380 MBchar*directly tov8::String::NewFromUtf8. V8 has an internal size assertion that fires at that scale, producing aSIGTRAP/EXC_BREAKPOINTthat kills the entire sidecar process.The fix in
apply_patch.tsis a simple pre-flight size check: if the file content exceeds 512 KB we skip diff generation entirely and store an empty string instead. This keeps SQLite rows at a safe size without losing any functional behavior (the deletion still happens; we just don't record a visual diff for it).The fix in
edit.ts'strimDiffis a matching early-return guard: if the diff string itself already exceeds 512 KB we return""immediately, before thesplit("\n")call that would allocate another equally large array. This protects every call site that goes throughtrimDiff, not just the delete path.How did you verify your code works?
Reproduced the crash locally: asked the AI to delete a 142 MB
.dmgfile viaapply_patch. Before the fix the sidecar hitSIGTRAPinsidev8::String::NewFromUtf8and the crash report showed the exact stack from the issue. After the fix the deletion completes cleanly, the sidecar stays up, and querying the session's parts returns normally.Screenshots / recordings
No UI change.
Checklist