-
Notifications
You must be signed in to change notification settings - Fork 772
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 edge panning crash and add warnings #3704
Conversation
/azp run create-installers |
Azure Pipelines successfully started running 1 pipeline(s). |
The requested installers were generated successfully. The installers are available at: Please be aware that these links are only valid for a limited time (~30 days). |
@youngminpark2559 Would you please try out the installer and run from terminal? You should see some warnings instead of a crash (hopefully). Please report them back. |
@rolandlo Sure. I'll update the version from 1.1.0 and run xournalpp in terminal to see crash warnings |
Just to make clear:You will have to use the installer provided above (in this comment). |
@rolandlo Yes, I installed the app through above link installer Ubuntu 20.04 |
@rolandlo I'm not sure if this warning message is related to crash error, I saw this warning I tried to reproduce crash error with the file which has a lot of screenshot images but crash warning message (edge panning) strangely didn't occur yet in terminal |
These warnings ( |
@rolandlo I haven't met crash warnings (Selection does not... or View does not ...) or crash even if I tried to run the app on the same files and environment where I encountered the crash. The other day, I updated xournalpp through Ubuntu's automatic update and I performed apt-get purge xournalpp to install the app through above linked installer. I'll report the edge panning warnings if I encounter them on terminal or icon execution |
@rolandlo I got the edge panning error message. And the app crashed as well as warning message. Hope this helps. And the log file contents (/home/young/.cache/xournalpp/errorlogs/errorlog.20220112-170604.log)
|
/azp run create-installers |
Azure Pipelines successfully started running 1 pipeline(s). |
Thanks for the testing, the warning message and the crash log. |
The requested installers were generated successfully. The installers are available at: Please be aware that these links are only valid for a limited time (~30 days). |
@rolandlo I installed through above installer And I got the crash but the warning message didn't appear For a potential hint for debugging, I maximally zoomed out and I pasted screenshot on to xournalpp paper and moved screenshot by mouse drag, and it seems like this situation easily creates crash. The log is as following
|
@rolandlo And I observed additional situation where the warning message of edge panning shows and no crash |
You got that message almost two minutes after starting |
@rolandlo Yes, but above message (#3704 (comment)) is the situation where I encountered the crash without edge panning warning on the new installed xournalpp. Since the crash was not captured by edge panning exception, could that crash have occured regardless of edge panning? Hope that report helps. |
@LittleHuba @bhennion Does anyone of you understand why |
I have only given this a glance, but yes, it could definitely be a data race issue ( I imagine it could be triggered by xournalpp/src/control/tools/EditSelection.cpp Lines 158 to 174 in 484b5cb
The Also I don't really understand how the crash is triggered. Do we have an identified reproducible input sequence that (even only sometimes) leads to the crash? |
@bhennion @rolandlo I don't know when the crash exactly happen. Following is the environment and task where I get the crash
|
Add a check for Nullpointer into the respective function and attach a debugger to the positive path of the check. Then try to trigger the bug. The backtrack in the debugger will tell you what constellation of threads causes it. |
To identify the issue, I think we should insert some debug prints in the destructor and
If we end up with a sequence like
this would help a lot towards identifying the cause. Also, I think there is an issue in xournalpp/src/control/tools/EditSelection.cpp Lines 727 to 730 in 484b5cb
should also insert g_source_destroy before the unref to cancel the source in the glib main loop (see this link).
|
This is blocking the 1.1.1 release, so for now, I will (in a few days) create a separate PR that allows edge panning to be disabled in the settings (and also incorporate the fix for the issue I identified above). Hopefully that will allow @youngminpark2559 and other people to work around the issue in the meantime. |
Closing in favor of #3776. |
Fixes #3618. Warnings have been included to detect the problem.