| Summary: | Crash on autosave (assert triggered for unbalanced unlock) | ||
|---|---|---|---|
| Product: | [Applications] krita | Reporter: | Friedrich W. H. Kossebau <kossebau> |
| Component: | General | Assignee: | Dmitry Kazakov <dimula73> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | Keywords: | drkonqi |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/krita/4a3058a511766761cea7a6b796e03f38c301cb12 | Version Fixed/Implemented In: | |
| Sentry Crash Report: | |||
|
Description
Friedrich W. H. Kossebau
2016-05-19 16:24:21 UTC
Seems I am the only one running Krita as "debug" build since May 6th, to hit asserts? :)
Not sure given I have no real experience with lock systems. The crash I hit was due to Q_ASSERT(locked()); in KisImage::unlock(). The reason is that KisDocument::Private::SafeSavingLocker uses a member KisImageBarrierLockAdapter m_imageLock. Which it explicitely unlocks again, to keep locking in balance, together with std::try_lock. Just, KisImageBarrierLockAdapter in its deconstructor unconditionally calls unlock on the image, not taking into account any state of other unlock/lock calls. Which seems strange to me (but perhaps that is some pattern in lock code writing?). At least the assert in KisImage::unlock() is expecting else.
As Lockable/BasicLockable seem to not require that unlock in the destructor, and the only current usecase of KisImageBarrierLockAdapterImpl explicitely cares for unlocking, this patch here makes the assert happy at least, by no longer doing unbalanced unlock calls:
diff --git a/libs/image/kis_image_barrier_lock_adapter.h b/libs/image/kis_image_barrier_lock_adapter.h
index 2f949e2..734381a 100644
--- a/libs/image/kis_image_barrier_lock_adapter.h
+++ b/libs/image/kis_image_barrier_lock_adapter.h
@@ -30,7 +30,6 @@ public:
}
inline ~KisImageBarrierLockAdapterImpl() {
- m_image->unlock();
}
inline void lock() {
lines 1-12/12 (END)
Git commit 4a3058a511766761cea7a6b796e03f38c301cb12 by Friedrich W. H. Kossebau. Committed on 20/05/2016 at 08:36. Pushed by kossebau into branch 'master'. Fix assert triggered for unbalanced unlock on (auto)save Summary: KisDocument::Private::SafeSavingLocker uses a member KisImageBarrierLockAdapter m_imageLock to do the locking of KisImage. Which it explicitely locks and unlocks again, to keep locking in balance, together with std::try_lock. But KisImageBarrierLockAdapter in its deconstructor unconditionally also calls unlock on the image, without taking into account any state of other unlock/lock calls. Which then triggers the Q_ASSERT(locked()); in KisImage::unlock(). Reviewers: dkazakov, #krita:_next Reviewed By: dkazakov, #krita:_next Differential Revision: https://phabricator.kde.org/D1645 M +0 -1 libs/image/kis_image_barrier_lock_adapter.h http://commits.kde.org/krita/4a3058a511766761cea7a6b796e03f38c301cb12 |