Fix segmentation fault with mutex locking #9
Conversation
So, the reason the app is crashing is actually a classic race condition. The best way to handle this is to put the lock inside the main importFromFile_Json function itself, rather than just in the async one. Even though asyncImportFromFile_Json is what kicks off the new thread, it’s really just a wrapper. If we only put the lock there, we’d still be in trouble if we ever called the regular import directly while an async task was already running in the background. By adding std::lock_guard<std::mutex> lock(mutex_); right at the start of importFromFile_Json, we make sure that no matter how it gets called, it always waits its turn before touching shared data like the items map or the undoHistory. Looking at the logs, it seems like the crash happened because one thread was trying to clear the items while another was right in the middle of adding "beta" or "alpha." They ended up stepping on each other's toes in memory, which is a total recipe for a segfault. If we place the lock right after the JSON file is read but before we do items.clear(), we're golden. This way, we don't block other threads while the computer is doing the slow work of reading from the disk, but we're fully protected the second we start changing the actual manager state. It’s the same logic used in addItem, so it keeps everything consistent and stops those random crashes during the threaded tests!
Fixed missing semicolon in lock_guard initialization.
|
Okay, but in my own section, everything is running smoothly with a heavy test suite running on it. I think you may be accessing incorrectly. I would like to have screenshots of the steps you took. And I hope you are working with the updated file. Sorry for taking too long to reply, I was really engaged. |
|
The reason app crashes only occasionally is that we are dealing with a "race condition," which is essentially a game of digital musical chairs. When you run the test, multiple threads are rushing to update the same list of items or register types at the exact same microsecond. Most of the time, the CPU manages to sequence them just far enough apart that they don't collide, and the program runs fine. However, every so often, two threads try to overwrite the exact same piece of memory at once, or one thread moves a list in memory while the other is still trying to read it. This creates a "collision" that confuses the system, causing it to panic and shut the program down with a segmentation fault to prevent further corruption. |
I got this error many time during the Testing the project.
|
|
Please merge this into the main repository once testing is complete. I have confirmed that the issue was not with the Google Test Suite, but rather a race condition caused by two threads accessing the same memory location simultaneously. The inconsistency in previous runs was due to thread scheduling; the segmentation fault only triggers when the threads overlap. |
|
Oh dear! I was able to see the error you are talking about. In my own case, all were passed. I would like to know the branch you're working with because you might be having issues on the branches and not the main. |
|
If you run ./TestApp multiple times, you'll start to see it crash. |
The platform that failed, I didn't set up for that. |
|
The main branch |
|
The main branch is working, and that is the first release of this project. I would advise you to clone the repo and rebuild the system, just following the steps provided in the README to build it |
So, the reason the app is crashing is actually a classic race condition. The best way to handle this is to put the lock inside the main importFromFile_Json function itself, rather than just in the async one. Even though asyncImportFromFile_Json is what kicks off the new thread, it’s really just a wrapper. If we only put the lock there, we’d still be in trouble if we ever called the regular import directly while an async task was already running in the background.
By adding std::lock_guardstd::mutex lock(mutex_); right at the start of importFromFile_Json, we make sure that no matter how it gets called, it always waits its turn before touching shared data like the items map or the undoHistory.
Looking at the logs, it seems like the crash happened because one thread was trying to clear the items while another was right in the middle of adding "beta" or "alpha." They ended up stepping on each other's toes in memory, which is a total recipe for a segfault. If we place the lock right after the JSON file is read but before we do items.clear(), we're golden. This way, we don't block other threads while the computer is doing the slow work of reading from the disk, but we're fully protected the second we start changing the actual manager state. It’s the same logic used in addItem, so it keeps everything consistent and stops those random crashes during the threaded tests!