Conversation
Reason: When calling QApplication::processEvents() all events are processed also user inputs which might lead to change the diff while we are reading the hunks which leads to a clear of the files where the fetchMore is currently working on it which is not correct and leads to undefined behaviour
|
Seems not to fix it yet |
| mFiles.size() < mDiffTreeModel->fileCount(dtw->selectedIndex()); | ||
| } | ||
|
|
||
| class Guard { |
There was a problem hiding this comment.
I've not observed this crash, but just to offer some thoughts based on the code changes. Presumably this involves two or more threads, which means you need to ensure data is synchronised between them (specifically synchronise L1 and L2 cache that might not be shared). The C++ way of solving that would be std::atomic_bool, which has load and store that will specify how to deal with the inter-thread synchronisation. There's more about this in https://en.cppreference.com/w/cpp/atomic/atomic.html and https://en.cppreference.com/w/cpp/atomic/memory_order.html
The second thing here is that I'd suggest not to define this class. I believe you can achieve the same effect by using std::mutex and instead of using a std::lock_guard or similar mechanisms, just use try_lock. If that fails, you can do mCancelFetchMore.store(false) (maybe with memory order specifically set), and post QApplication::processEvents you can unlock and check with mCancelFetchMore.load() (which also might need a memory order set)
There was a problem hiding this comment.
I thought at the beginning as well that two or more threads are involved, but there aren't.
When I call QApplication::processEvents() the current loop will be interrupted and mouse events are handled. This mouse event triggeres then again a new diff (when clicking on another element in the commitlist) which triggers the fetch.
The problem is that you cannot wait until the previous fetch is finished because it will never, because everything runs in a single thread so we have to return to go on in the fetch loop.
This is reproducable when you have a large number of hunks in a diff which must be loaded.
The Guard was just there to release the lock after the function returns so it cannot be forgotten.
But this actually does not solve the problem yet so I have to further investigate
Reason: When calling QApplication::processEvents() all events are processed also user inputs which might lead to change the diff while we are reading the hunks which leads to a clear of the files where the fetchMore is currently working on it which is not correct and leads to undefined behaviour