| Summary: | Crash loading incorrect file (backup file) as pattern resource. | ||
|---|---|---|---|
| Product: | [Applications] krita | Reporter: | wolthera <griffinvalley> |
| Component: | General | Assignee: | Tiar <tamtamy.tymona> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | amy, griffinvalley, tamtamy.tymona |
| Priority: | NOR | Keywords: | drkonqi, regression, release_blocker |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Neon | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
New crash information added by DrKonqi
Potential patch for tasksets |
||
|
Description
wolthera
2021-11-04 14:27:45 UTC
Created attachment 143202 [details]
New crash information added by DrKonqi
krita (5.0.0-beta2 (git e610fb6)) using Qt 5.15.3
Also having this with tasksets, this time, I selected "taskset.kts copy"
-- Backtrace (Reduced):
#4 0x00007f22cb8b6bef in KisResourceLoaderBase::load(QString const&, QIODevice&, QSharedPointer<KisResourcesInterface>) (resourcesInterface=..., dev=..., name=..., this=0x0) at /home/wolthera/krita/src/libs/resources/KisResourceLoader.h:97
#5 KisResourceLocator::importResourceFromFile(QString const&, QString const&, bool, QString const&) (this=0x5606b6364470, resourceType=..., fileName=..., allowOverwrite=<optimized out>, storageLocation=...) at /home/wolthera/krita/src/libs/resources/KisResourceLocator.cpp:305
#6 0x00007f22cb8cc82f in KisAllResourcesModel::importResourceFile(QString const&, bool, QString const&) (this=0x5606bc949d50, filename=..., allowOverwrite=<optimized out>, storageId=...) at /home/wolthera/krita/src/libs/resources/KisResourceModel.cpp:447
#7 0x00007f22cb8c9dcf in KisResourceModel::importResourceFile(QString const&, bool, QString const&) (this=this@entry=0x7ffc2221cbb0, filename=..., allowOverwrite=allowOverwrite@entry=false, storageId=...) at /home/wolthera/krita/src/libs/resources/KisResourceModel.cpp:715
#8 0x00007f22c9595455 in KisResourceUserOperations::importResourceFileWithUserInput(QWidget*, QString, QString, QString) (widgetParent=widgetParent@entry=0x5606bc9684b0, storageLocation=..., resourceType=..., resourceFilepath=...) at /home/wolthera/krita/src/libs/resourcewidgets/KisResourceUserOperations.cpp:73
Created attachment 143220 [details]
Potential patch for tasksets
@Wolthera, how did you make `sketch.png~` to be visible for you when you search for patterns? I can't see it when the ending is wrong. I think I have the Qt dialogs, I tried to enable native dialogs but even after restart it still shows the same dialog.
The pattern crash probably happens because:
- it needs md5 sum. But it isn't calculated yet
- to calculate md5 sum, it tries to save the pattern to memory (why? it's not embedded in anything, it's a file)
- saving pattern to memory doesn't work, because it uses: QImage.save(buffer, "PNG~") function. So it doesn't save any bytes
- md5 doesn't work on empty buffers (no bytes saved)
Since we're going to talk about md5 situation with Dmitry, so I'll mention that.
For tasksets, the backtrace is different (note that it doesn't go into `md5Sum()` function but already crashes on load()). However I have the same problem, reproducing your issue :/
However I read the code and I think the problem might be that it simply doesn't create a loader (I can't be sure because the backtrace is cut, I think - was it segmentation fault because of empty loader? I hope so, at least). In that case, I provided a patch file, which might or might not work, because I couldn't test it. It would be awesome if you could please test it, if it's possible.
I cannot reproduce either of the two crashes with de586e5385 under MSVC. (In reply to Tiar from comment #2) > Created attachment 143220 [details] > Potential patch for tasksets > > @Wolthera, how did you make `sketch.png~` to be visible for you when you > search for patterns? I can't see it when the ending is wrong. I think I have > the Qt dialogs, I tried to enable native dialogs but even after restart it > still shows the same dialog. > > The pattern crash probably happens because: > - it needs md5 sum. But it isn't calculated yet > - to calculate md5 sum, it tries to save the pattern to memory (why? it's > not embedded in anything, it's a file) > - saving pattern to memory doesn't work, because it uses: > QImage.save(buffer, "PNG~") function. So it doesn't save any bytes > - md5 doesn't work on empty buffers (no bytes saved) > Since we're going to talk about md5 situation with Dmitry, so I'll mention > that. > > For tasksets, the backtrace is different (note that it doesn't go into > `md5Sum()` function but already crashes on load()). However I have the same > problem, reproducing your issue :/ > However I read the code and I think the problem might be that it simply > doesn't create a loader (I can't be sure because the backtrace is cut, I > think - was it segmentation fault because of empty loader? I hope so, at > least). In that case, I provided a patch file, which might or might not > work, because I couldn't test it. It would be awesome if you could please > test it, if it's possible. I am using the KDE native dialogs. This patch fixes the crash with the task set, but not the crash with the backup file. Git commit e1be9329dc5210170c95a55488c051546388dc63 by Agata Cacko. Committed on 08/11/2021 at 12:04. Pushed by tymond into branch 'master'. Fix crash on loading `tmp.kts copy` as a taskset Before this commit, if one tried to use native file dialogs and import a file called for example `tmp.kts copy` as a taskset, Krita would crash or assert. This commit ensures that Krita recognizes that the loader hasn't been created and leaves the function early, which avoids the crash. M +5 -0 libs/resources/KisResourceLocator.cpp https://invent.kde.org/graphics/krita/commit/e1be9329dc5210170c95a55488c051546388dc63 Git commit 091a6db6b710fbc9818fc1299b26c9390f9b63c4 by Agata Cacko. Committed on 08/11/2021 at 15:49. Pushed by tymond into branch 'krita/5.0'. Fix crash on loading `tmp.kts copy` as a taskset Before this commit, if one tried to use native file dialogs and import a file called for example `tmp.kts copy` as a taskset, Krita would crash or assert. This commit ensures that Krita recognizes that the loader hasn't been created and leaves the function early, which avoids the crash. M +5 -0 libs/resources/KisResourceLocator.cpp https://invent.kde.org/graphics/krita/commit/091a6db6b710fbc9818fc1299b26c9390f9b63c4 Fix for tasksets works for patterns too, at least in my case. @Wolthera, if you can still reproduce it, please add new steps to reproduce (unless they are completely the same), or maybe the file you try to import (though here it fails on creating a loader for the filename so I don't know why it would crash for you). On my system, with some debug lines added, I get this output: Could not create loader for "patterns" "pattern.png~" "application/x-trash" Could not create loader for "patterns" "pattern.png~" "application/x-trash" Could not retrieve imported resource from the storage "patterns" "/home/tymon/kritadev/otherfiles/testfiles/resources/pattern.png~" "" Failed to import resource "/home/tymon/kritadev/otherfiles/testfiles/resources/pattern.png~" First attempt: isNull = true Storage location: "" marked as existing already: true Could not create loader for "patterns" "pattern.png~" "application/x-trash" Failed to import resource "/home/tymon/kritadev/otherfiles/testfiles/resources/pattern.png~" I get two messages, first asking if I want to overwrite the file (which is clearly a bug - but it doesn't happen with correct importing), and then that it failed importing. I know why it does that... I will make a bug report for that tomorrow (so it will be more clear than this long one here). Yep, you're correct, this is fixed :) |