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 period backup crash due to the dead lock of std::lock_guard (2nd Solution) #14917

Closed
wants to merge 3 commits into from

Conversation

donho
Copy link
Member

@donho donho commented Mar 28, 2024

The crash occurs because the thread terminates the task prematurely due to PostMessage's nature, so FileManager::backupCurrentBuffer() is run always by the main thread - it makes the line std::lock_guard<std::mutex> lock(backup_mutex); has the dead lock on the 2nd main thread's entry and cause the crash:
https://en.cppreference.com/w/cpp/thread/mutex/lock

Using SendMessage instead of PostMessage to ensure that thread keeps the mutex until entire operation is terminated.

This solution seems better than #14914 because for the following reasons:

  1. The old solution (Fix period backup crash due to the dead lock of std::lock_guard #14914) has no lock at all: though it doesn't crash while the main thread enters the 2nd time into FileManager::backupCurrentBuffer() (with write method blocked), the goal of the lock is to prevent the race condition. Whereas this PR prevents the 2nd thread from entering into FileManager::backupCurrentBuffer(), until the 1st thread finish its task.
  2. Keep using standard C++11 function.

Fix #14906

@donho
Copy link
Member Author

donho commented Mar 28, 2024

@xomx
Please check this PR.

@donho
Copy link
Member Author

donho commented Mar 29, 2024

@xomx
Since this PR has fixed the pontential crash #14906, I have to merge this PR for the long waiting new release.
Please feel free for any reviewing/suggestion of this PR for the future release.

@donho donho closed this in bbeaafa Mar 29, 2024
@donho donho deleted the fix_backup_dead_lock branch March 29, 2024 15:15
@xomx
Copy link
Contributor

xomx commented Mar 29, 2024

@donho

Sorry for my unresponsiveness - many IRL stuff + I am ill.

Seems this is not the right solution, I am still able to crash it at the N++ exit from the same reason:

npp_backup_mutex_deadlock_exception

I cannot do much more now, but I will try ASAP.

@donho
Copy link
Member Author

donho commented Mar 29, 2024

@xomx
Damn! Hope you'll get recovered soon!

If it's possible, could you just provide me the instructions to reproduce the crash please?
Otherwise, no problem - it may not fix the crash, it's not getting worse anyway.

Bon rétablissement !

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.

possible crash when recursively entering the Notepad++ backup_mutex
2 participants