Closed sahensley closed 2 years ago
pbek
commented
2 years ago I can't reproduce a crash with 21.11.9 so far on Linux or macOS. So the application doesn't crash for you in 21.11.8? It could have something to do with #2303 (@Waqar144?).
Can you please post the output from the debug settings that you can copy when you get into the settings dialog in QOwnNotes and head over to the Debug section of it.
Do you have the same issue when running QOwnNotes in a new session (see cli-parameters)?
Waqar144
commented
2 years ago I can't reproduce a crash with 21.11.9 so far on Linux or macOS. So the application doesn't crash for you in 21.11.8? It could have something to do with #2303 (@Waqar144?).
Most likely, yes. Maybe something we are not guarding against and it just surfaced now.
Can you try this on linux?
coredumpctl debugIf that doesn't work, try running QON from the terminal? gdb ./QON and make it crash. Once it crashes (will freeze when in gdb), go back to the terminal and type bt. Copy paste the output here.
And if that doesn't help, I think we can just revert the PR till we can find what the cause is.
pbek
commented
2 years ago So far I didn't manage to start with an empty note edit panel in 21.11.9...
Waqar144
commented
2 years ago Me too, but there are too many settings combinations so could be because of that.
sahensley
commented
2 years ago I'll create a new user on my personal laptop and grab that data tonight. The good news is, this bug is obscure enough that I don't think too many people will be impacted :grin:
pbek
commented
2 years ago And maybe it's just one person. 😉
Waqar144
commented
2 years ago The good news is, this bug is obscure enough
That's very bad news to me as a developer 😂
sahensley
commented
2 years ago I created a new system user, downloaded QON, went through the settings with default options and I could not replicate the issue until I introduced a plugin.
Once I added the @tag tagging in note text (experimental) plugin and restarted, the blank note panel upon launch and crash when double-clicking the highlighted note as the first action appeared. The crash only started after 21.9.11 but it appears to be tied to this plugin.
I'll leave the issue open incase there is anything you want to test or investigate but feel free to close it as this appears to be isolated to the plugin.
pbek
commented
2 years ago Thank you, I can reproduce it now... I can fix the crash, but there are still things broken with the script on.
Waqar144
commented
2 years ago Still doesn't crash for me even with the script enabled.
pbek
commented
2 years ago I get the empty note panel, there seems to be no current note... Then there is a crash in MainWindow::on_noteEditTabWidget_currentChanged(int index), but fixing the crash doesn't help with the empty note edit panel.
Waqar144
commented
2 years ago the only way that could crash is if there is no currentWidget, so maybe we need to ensure we always have one during startup when we know that the notes are now available.
But besides that, enabling the script slows down the QON startup time by several second to a minute. And one reason for this is that our notes may contain images etc which are also grepped for tags by the script. We can atleast ignore those.
pbek
commented
2 years ago I don't think the issue is the crash (it can be fixed easily), it's more like the issue is that no current note is assigned.
pbek
commented
2 years ago Of course that doesn't fix the underlying issue.
pbek
commented
2 years ago There now is a new release, could you please test it and report if it works for you?
Waqar144
commented
2 years ago Can it be that the notes weren't loaded yet and so there was no current note?
pbek
commented
2 years ago Yes, that maybe could be the issue. But it's still a timing problem. Because the there has to be a currentNote when the notes are finally loaded and shown. I also (like above) get an empty note edit with all notes loaded.
pbek
commented
2 years ago I just bisected the issue... It was introduced with #2303, @Waqar144 😉
pbek
commented
2 years ago And it's:
QTimer::singleShot(10, this, [this]{
updatePanelsSortOrder();
});That was not my first suspect... just my 2nd
pbek
commented
2 years ago updatePanelsSortOrder() does a loadNoteDirectoryList();. If I put that above below the singleshot it works... but I don't think it's worth to do that twice in that place... I'll look more into it.
pbek
commented
2 years ago If I just disable the handleScriptingNotesTagUpdating() call in MainWindow::reloadTagTree() it also works with the single shot...
Waqar144
commented
2 years ago If I just disable the
handleScriptingNotesTagUpdating()call inMainWindow::reloadTagTree()it also works with the single shot...
it doesn't in reality. That just makes things faster so you have a lower chance of hitting the issue ;)
pbek
commented
2 years ago Uhhh, it's the directoryWatcherWorkaround in handleScriptingNotesTagUpdating... It's veeeerrry bad idea to call directoryWatcherWorkaround asynchronously, because it blocks the directory watcher at some random time (and not at some predefined time).
pbek
commented
2 years ago We shouldn't call anything with directoryWatcherWorkaround in it (like updatePanelsSortOrder()) with singleshot during the "boot up process"... 😅
pbek
commented
2 years ago Of course I could make a workaround by testing if tagging is handled by a script with ScriptingService::instance()->noteTaggingHookExists() and decide whether to use singleshot or not... (along with adding a lot of explanation 😅 )
Waqar144
commented
2 years ago How about we create a postInit() method and handle all delayed stuff there in one singleshot just. that way we can ensure some order between things and get the benefits as well. In the constructor itself we can do the bare minimum to get up and running.
pbek
commented
2 years ago Yeah, in this case I didn't have the issue when I just moved the singleshot code block a bit down... But I guess at the end of the initializer is the safest place
pbek
commented
2 years ago Meh, it was a fluke, it also doesn't work at the end of the initializer with singleShot.
pbek
commented
2 years ago It only works if I increase the delay time to 1 sec, that's long... maybe I really check for the tagging hooks.
Waqar144
commented
2 years ago yep, proper solution is only to ensure we load notes before doing anything.
pbek
commented
2 years ago Waqar144
commented
2 years ago https://github.com/pbek/QOwnNotes/commit/722b00a98d102a7aa1390fbae710d7387a8e7ac4 I don't think this is a good solution really. Its complexity for no reason and doesn't fix the underlying issue.
pbek
commented
2 years ago Suggestions?
pbek
commented
2 years ago doesn't fix the underlying issue
which issue? there are multiple 😅 e.g. that QSignalBlocker blocker(this->noteDirectoryWatcher) doesn't seem to block all file system events... thus the need for directoryWatcherWorkaround
Waqar144
commented
2 years ago We need to ensure proper order between things as I understand it. So either we do that, which will require carefully checking things/function calls. Or we can just do the loadNotes() directly, no need to delay it and then the current note will be there.
pbek
commented
2 years ago The order was (most probably) proper before doing things asynchronously... I guess we never can be sure...
Waqar144
commented
2 years ago So, imo it would be best to just revert that change or that part of change.
pbek
commented
2 years ago You mean not loading updatePanelsSortOrder() with singleShot at all?
pbek
commented
2 years ago Meanwhile, @sahensley... There now is a new release, could you please test it and report if it works for you?
sahensley
commented
2 years ago Oh wow, I certainly uncovered something with this issue :sweat_smile:
With 21.11.10, the issue still occurred. With 21.11.11, the issue has been resolved :tada:
I have the "tag in note" plugin enabled without any problems AND startup seems faster as well.
Thank you both @pbek and @Waqar144 !!
pbek
commented
2 years ago Great, thank you for testing!
With 21.11.10, the issue still occurred.
But no Segfault, right?
sahensley
commented
2 years ago Ah yes, good point.
No segfault on 21.11.10, the note editor panel would appear blank like the second screenshot in the initial issue. Then if the default highlighted note was double-clicked, the panel would go grey. Then double-clicking again would go back to the blank appearance. Clicking any other note than the one highlighted by default would solve that problem in 21.11.10 but it's great to see how 21.11.11 that isn't required at all.
pbek
commented
2 years ago Ok, thank you. 😉
Expected behaviour
The content is opened in a new tab.
Actual behaviour
The application crashes. When launching from the CLI - the error which goes to the console is a segfault.
Steps to reproduce
This issue happens in very specific circumstances. I swear I'm not trying to model my workflow after this appropriate XKCD 😄
For reference, this screenshot has been taken immediately after launching 21.11.8. The last note I used - "_scratchpad" - loads into the main editor panel.
This is a screenshot immediately after launching 21.11.9. Notice how the main editor panel is blank. I can trigger a crash (no error dialog) by double-clicking on the note which is highlighted by default in the Note List panel, which is the _scratchpad note in this instance. I can completely avoid a crash by clicking on any other note, which then puts text into the editor panel. Once the panel has content in it, I can go back to the initial note of the session without issue.
If QON has the option to "Restore open notes.." and tabs were open when the application was closed, QON will not crash when double-clicking the note highlighted upon launch.
So to summarize the very specific circumstances:
I verified this happens on both the Linux AppImage and Windows install of 21.11.9.
Let me know if you run into any issues replicating the crash, I can record a video of some sort. Thanks again! 😄