Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: deadlock if error handler jumps somewhere else #1647

Merged
merged 4 commits into from Oct 21, 2023

Conversation

takase1121
Copy link
Member

This PR complements #1599.

The root issue is that the check callback runs within a critical section. It is very possible to longjmp out of it via errors. This PR allows errors to be caught by an error handler. This prevents an error from escaping the critical section and cause a deadlock.

Likely fixes #1531, #1532 (maybe #1556 too?).

Copy link
Member

@Guldoman Guldoman left a comment

Choose a reason for hiding this comment

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

Looks good, but I guess a review from @adamharrison is needed, as it's his territory.

src/api/dirmonitor.c Show resolved Hide resolved
docs/api/dirmonitor.lua Outdated Show resolved Hide resolved
docs/api/dirmonitor.lua Outdated Show resolved Hide resolved
docs/api/dirmonitor.lua Outdated Show resolved Hide resolved
Co-authored-by: Guldoman <giulio.lettieri@gmail.com>
@Guldoman Guldoman merged commit 1620a92 into lite-xl:master Oct 21, 2023
1 check passed
Bugfix Release Tracker automation moved this from PR to TODO Oct 21, 2023
takase1121 added a commit to takase1121/lite-xl that referenced this pull request Oct 24, 2023
…-xl#1647)

* fix: deadlock if error handler jumps somewhere else

* docs(dirmonitor): fix wrong data type in error callback

* docs(dirmonitor): clarify coroutines and deadlocks

* docs(dirmonitor): wording

Co-authored-by: Guldoman <giulio.lettieri@gmail.com>

---------

Co-authored-by: Guldoman <giulio.lettieri@gmail.com>
@takase1121 takase1121 moved this from TODO to Cherry Picked in Bugfix Release Tracker Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Error on first launch
2 participants