Skip to content

Fix crash when buffer is modified before callback fires#40

Open
benthamite wants to merge 3 commits intoemacs-languagetool:masterfrom
benthamite:fix/guard-stale-buffer-positions
Open

Fix crash when buffer is modified before callback fires#40
benthamite wants to merge 3 commits intoemacs-languagetool:masterfrom
benthamite:fix/guard-stale-buffer-positions

Conversation

@benthamite
Copy link
Contributor

Summary

This is a companion to #39. That PR guarded against the source buffer being killed before the url-retrieve callback fires; this one guards against the buffer being modified (i.e. still alive, but with different contents).

  • flycheck-languagetool--check-all: skip matches whose offset + length falls outside the current buffer bounds instead of signaling args-out-of-range
  • flycheck-languagetool--read-results: move the condition-case to wrap the entire callback invocation, fixing two issues:
    1. The error handler's return value previously fell through to mapcar, causing "Wrong type argument: sequencep"
    2. callback could be invoked twice (once with 'errored, once with 'finished)

Together with #39, this covers all three async race conditions in the callback: buffer killed, buffer modified, and response parse failure.

Problem

When the source buffer is edited while the LanguageTool HTTP request is in flight, the byte offsets in the JSON response can exceed (point-max). This causes:

Error from syntax checker languagetool: Args out of range: 242, 1, 1
mapcar: Wrong type argument: sequencep, 69

The args-out-of-range error in --check-all is caught by the condition-case error handler, which calls (funcall callback 'errored ...). But because the condition-case was placed inside the mapcar argument, the handler's return value is passed to mapcar as a sequence — triggering the second error. With debug-on-error active, this enters the Emacs debugger in a recursive edit, freezing the UI.

Test plan

Includes 9 ERT tests covering:

  • Valid matches within buffer bounds are returned correctly
  • Matches with offset past point-max are silently skipped
  • Matches with offset + length past point-max are silently skipped
  • Mix of valid and stale matches: only valid ones returned
  • Empty matches list returns nil
  • Multiline buffer positions produce correct line numbers
  • Dead source buffer → callback called once with 'interrupted
  • Malformed response → callback called exactly once with 'errored
  • Well-formed response → callback called once with 'finished

@mavit
Copy link
Contributor

mavit commented Feb 24, 2026

Thanks.

Could you split the flycheck-languagetool--read-results fix into a separate commit to the flycheck-languagetool--check-all changes?

Could you plumb the tests into the Eask config? Is it feasible to run them as GitHub CI tests?

Restructure the `condition-case' in `--read-results' to wrap the
entire callback invocation.  Previously the error handler's return
value fell through to `mapcar' (causing "Wrong type argument:
sequencep") and callback could be invoked twice (once with
'errored, once with 'finished).

Include ERT tests covering callback semantics (dead source buffer,
error handling, and successful parse).
Skip matches whose offsets fall outside the current buffer bounds
instead of signaling `args-out-of-range'.  The buffer may have been
modified while the LanguageTool request was in flight.

Include ERT tests for --check-all covering valid matches, out-of-bounds
offsets, mixed results, empty matches, and multiline positions.
Update the Eask test script to run ERT tests and add the test
target to the Makefile ci recipe so tests run in GitHub Actions.
@benthamite benthamite force-pushed the fix/guard-stale-buffer-positions branch from 7c846fd to 46b32d0 Compare February 24, 2026 15:05
@benthamite
Copy link
Contributor Author

Yes, done.

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