Bug 254545 - Crash on changing layer visibility
Summary: Crash on changing layer visibility
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: General (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR crash
Target Milestone: ---
Assignee: Dmitry Kazakov
URL:
Keywords:
: 219503 254394 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-10-18 15:30 UTC by Sven Langkamp
Modified: 2010-12-14 18:52 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sven Langkamp 2010-10-18 15:30:59 UTC
Application: krita (2.3 Beta 2)
KDE Platform Version: 4.4.5 (KDE 4.4.5) (Compiled from sources)
Qt Version: 4.6.3
Operating System: Linux 2.6.32-25-generic x86_64
Distribution: Ubuntu 10.04.1 LTS

-- Information about the crash:
I changed the visibility of a layer when it crashed. There was a transparency mask applied to the layer.

 -- Backtrace:
Application: Krita (krita), signal: Segmentation fault
[Current thread is 1 (Thread 0x7f6e144cd800 (LWP 17708))]

Thread 8 (Thread 0x7f6e016b1710 (LWP 17709)):
#0  0x00007f6e1e70bfe3 in select () at ../sysdeps/unix/syscall-template.S:82
#1  0x00007f6e25e2c93e in qt_safe_select(int, fd_set*, fd_set*, fd_set*, timeval const*) () from /usr/lib/libQtCore.so.4
#2  0x00007f6e25e31f1d in QEventDispatcherUNIXPrivate::doSelect(QFlags<QEventLoop::ProcessEventsFlag>, timeval*) () from /usr/lib/libQtCore.so.4
#3  0x00007f6e25e32d0b in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQtCore.so.4
#4  0x00007f6e25e046c2 in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQtCore.so.4
#5  0x00007f6e25e04a9c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQtCore.so.4
#6  0x00007f6e25d138db in QThread::exec() () from /usr/lib/libQtCore.so.4
#7  0x00007f6e25de4dd8 in ?? () from /usr/lib/libQtCore.so.4
#8  0x00007f6e25d15f95 in ?? () from /usr/lib/libQtCore.so.4
#9  0x00007f6e25a859ca in start_thread (arg=<value optimized out>) at pthread_create.c:300
#10 0x00007f6e1e7136fd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#11 0x0000000000000000 in ?? ()

Thread 7 (Thread 0x7f6e00ca9710 (LWP 17714)):
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162
#1  0x00007f6e25d16fdb in QWaitCondition::wait(QMutex*, unsigned long) () from /usr/lib/libQtCore.so.4
#2  0x00007f6e25d12d64 in QSemaphore::acquire(int) () from /usr/lib/libQtCore.so.4
#3  0x00007f6e250e5348 in KisTileDataPooler::waitForWork (this=0x2c95aa0) at /home/sven/kde/src/koffice/krita/image/tiles3/kis_tile_data_pooler.cc:127
#4  0x00007f6e250e53b2 in KisTileDataPooler::run (this=0x2c95aa0) at /home/sven/kde/src/koffice/krita/image/tiles3/kis_tile_data_pooler.cc:156
#5  0x00007f6e25d15f95 in ?? () from /usr/lib/libQtCore.so.4
#6  0x00007f6e25a859ca in start_thread (arg=<value optimized out>) at pthread_create.c:300
#7  0x00007f6e1e7136fd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#8  0x0000000000000000 in ?? ()

Thread 6 (Thread 0x7f6dfbfff710 (LWP 17715)):
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162
#1  0x00007f6e25d16fdb in QWaitCondition::wait(QMutex*, unsigned long) () from /usr/lib/libQtCore.so.4
#2  0x00007f6e25d12b84 in QSemaphore::tryAcquire(int, int) () from /usr/lib/libQtCore.so.4
#3  0x00007f6e2510934e in KisTileDataSwapper::waitForWork (this=0x2c95ad0) at /home/sven/kde/src/koffice/krita/image/tiles3/swap/kis_tile_data_swapper.cpp:84
#4  0x00007f6e25109368 in KisTileDataSwapper::run (this=0x2c95ad0) at /home/sven/kde/src/koffice/krita/image/tiles3/swap/kis_tile_data_swapper.cpp:90
#5  0x00007f6e25d15f95 in ?? () from /usr/lib/libQtCore.so.4
#6  0x00007f6e25a859ca in start_thread (arg=<value optimized out>) at pthread_create.c:300
#7  0x00007f6e1e7136fd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#8  0x0000000000000000 in ?? ()

Thread 5 (Thread 0x7f6dfb7fe710 (LWP 17728)):
[KCrash Handler]
#5  0x00007f6e1f121423 in __dynamic_cast () from /usr/lib/libstdc++.so.6
#6  0x00007f6e25179e1f in KisGroupLayer::tryObligeChild (this=0x2b51b80) at /home/sven/kde/src/koffice/krita/image/kis_group_layer.cc:119
#7  0x00007f6e2517a06d in KisGroupLayer::original (this=0x2b51b80) at /home/sven/kde/src/koffice/krita/image/kis_group_layer.cc:145
#8  0x00007f6e250dd874 in KisAsyncMerger::setupProjection (this=0x13b6368, currentNode=..., rect=..., useTempProjection=false)
    at /home/sven/kde/build/koffice/krita/image/../../../../src/koffice/krita/image/kis_async_merger.h:242
#9  0x00007f6e250dd334 in KisAsyncMerger::startMerge (this=0x13b6368, walker=...) at /home/sven/kde/build/koffice/krita/image/../../../../src/koffice/krita/image/kis_async_merger.h:193
#10 0x00007f6e250ddfc3 in KisUpdateJobItem::run (this=0x13b6340) at /home/sven/kde/build/koffice/krita/image/../../../../src/koffice/krita/image/kis_updater_context.h:39
#11 0x00007f6e25d0be7f in ?? () from /usr/lib/libQtCore.so.4
#12 0x00007f6e25d15f95 in ?? () from /usr/lib/libQtCore.so.4
#13 0x00007f6e25a859ca in start_thread (arg=<value optimized out>) at pthread_create.c:300
#14 0x00007f6e1e7136fd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#15 0x0000000000000000 in ?? ()

Thread 4 (Thread 0x7f6dfaffd710 (LWP 17729)):
#0  pthread_cond_timedwait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S:211
#1  0x00007f6e25d16f42 in QWaitCondition::wait(QMutex*, unsigned long) () from /usr/lib/libQtCore.so.4
#2  0x00007f6e25d0bf21 in ?? () from /usr/lib/libQtCore.so.4
#3  0x00007f6e25d15f95 in ?? () from /usr/lib/libQtCore.so.4
#4  0x00007f6e25a859ca in start_thread (arg=<value optimized out>) at pthread_create.c:300
#5  0x00007f6e1e7136fd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#6  0x0000000000000000 in ?? ()

Thread 3 (Thread 0x7f6dfa7fc710 (LWP 17730)):
[KCrash Handler]
#5  0x00007f6e1f121423 in __dynamic_cast () from /usr/lib/libstdc++.so.6
#6  0x00007f6e25179e1f in KisGroupLayer::tryObligeChild (this=0x2b51b80) at /home/sven/kde/src/koffice/krita/image/kis_group_layer.cc:119
#7  0x00007f6e2517a06d in KisGroupLayer::original (this=0x2b51b80) at /home/sven/kde/src/koffice/krita/image/kis_group_layer.cc:145
#8  0x00007f6e25193baa in KisLayer::projection (this=0x2b51b80) at /home/sven/kde/src/koffice/krita/image/kis_layer.cc:467
#9  0x00007f6e251833e1 in KisImage::projection (this=0x2b65b60) at /home/sven/kde/src/koffice/krita/image/kis_image.cc:636
#10 0x00007f6e2572a2d3 in KisTextureTileUpdateInfo::retrieveData (this=0x7f6dfa7fb6d0, image=...) at /home/sven/kde/src/koffice/krita/ui/opengl/kis_texture_tile_update_info.h:52
#11 0x00007f6e25727df2 in KisOpenGLImageTextures::updateCache (this=0x55d2500, rect=...) at /home/sven/kde/src/koffice/krita/ui/opengl/kis_opengl_image_textures.cpp:229
#12 0x00007f6e255cb94e in KisCanvas2::startUpdateCanvasProjection (this=0x51e2590, rc=...) at /home/sven/kde/src/koffice/krita/ui/canvas/kis_canvas2.cpp:452
#13 0x00007f6e255ccf2a in KisCanvas2::qt_metacall (this=0x51e2590, _c=QMetaObject::InvokeMetaMethod, _id=7, _a=0x7f6dfa7fb9b0) at /home/sven/kde/build/koffice/krita/ui/kis_canvas2.moc:120
#14 0x00007f6e25e19036 in QMetaObject::activate(QObject*, QMetaObject const*, int, void**) () from /usr/lib/libQtCore.so.4
#15 0x00007f6e25187d9f in KisImage::sigImageUpdated (this=0x2b65b60, _t1=...) at /home/sven/kde/build/koffice/krita/image/kis_image.moc:173
#16 0x00007f6e2518724b in KisImage::slotProjectionUpdated (this=0x2b65b60, rc=...) at /home/sven/kde/src/koffice/krita/image/kis_image.cc:1108
#17 0x00007f6e25187a6f in KisImage::qt_metacall (this=0x2b65b60, _c=QMetaObject::InvokeMetaMethod, _id=16, _a=0x7f6dfa7fbbb0) at /home/sven/kde/build/koffice/krita/image/kis_image.moc:136
#18 0x00007f6e25e19036 in QMetaObject::activate(QObject*, QMetaObject const*, int, void**) () from /usr/lib/libQtCore.so.4
#19 0x00007f6e250dad51 in KisUpdaterContext::sigContinueUpdate (this=0x13b5f98, _t1=...) at /home/sven/kde/build/koffice/krita/image/moc_kis_updater_context.cpp:179
#20 0x00007f6e250daccd in KisUpdaterContext::qt_metacall (this=0x13b5f98, _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0x7f6dfa7fbd10)
    at /home/sven/kde/build/koffice/krita/image/moc_kis_updater_context.cpp:164
#21 0x00007f6e25e19036 in QMetaObject::activate(QObject*, QMetaObject const*, int, void**) () from /usr/lib/libQtCore.so.4
#22 0x00007f6e250dab5f in KisUpdateJobItem::sigContinueUpdate (this=0x2ca0f60, _t1=...) at /home/sven/kde/build/koffice/krita/image/moc_kis_updater_context.cpp:91
#23 0x00007f6e250ddffd in KisUpdateJobItem::run (this=0x2ca0f60) at /home/sven/kde/build/koffice/krita/image/../../../../src/koffice/krita/image/kis_updater_context.h:42
#24 0x00007f6e25d0be7f in ?? () from /usr/lib/libQtCore.so.4
#25 0x00007f6e25d15f95 in ?? () from /usr/lib/libQtCore.so.4
#26 0x00007f6e25a859ca in start_thread (arg=<value optimized out>) at pthread_create.c:300
#27 0x00007f6e1e7136fd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#28 0x0000000000000000 in ?? ()

Thread 2 (Thread 0x7f6df247c710 (LWP 17731)):
#0  pthread_cond_timedwait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S:211
#1  0x00007f6e25d16f42 in QWaitCondition::wait(QMutex*, unsigned long) () from /usr/lib/libQtCore.so.4
#2  0x00007f6e25d0bf21 in ?? () from /usr/lib/libQtCore.so.4
#3  0x00007f6e25d15f95 in ?? () from /usr/lib/libQtCore.so.4
#4  0x00007f6e25a859ca in start_thread (arg=<value optimized out>) at pthread_create.c:300
#5  0x00007f6e1e7136fd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#6  0x0000000000000000 in ?? ()

Thread 1 (Thread 0x7f6e144cd800 (LWP 17708)):
#0  0x00007f6e1e706f83 in *__GI___poll (fds=<value optimized out>, nfds=<value optimized out>, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:87
#1  0x00007f6e1b69f29a in ?? () from /usr/lib/libxcb.so.1
#2  0x00007f6e1b69f7d7 in ?? () from /usr/lib/libxcb.so.1
#3  0x00007f6e1b69fa85 in xcb_writev () from /usr/lib/libxcb.so.1
#4  0x00007f6e23f5a6fa in _XSend () from /usr/lib/libX11.so.6
#5  0x00007f6e23f5a839 in _XReply () from /usr/lib/libX11.so.6
#6  0x00007f6e23f4e2c3 in XSync () from /usr/lib/libX11.so.6
#7  0x00007f6e26380db7 in ?? () from /usr/lib/fglrx/libGL.so.1
#8  0x00007f6df3a289ac in ?? () from /usr/lib/fglrx/dri/fglrx_dri.so
#9  0x00007f6df3a28e6c in ?? () from /usr/lib/fglrx/dri/fglrx_dri.so
#10 0x00007f6df3a2d3c6 in ?? () from /usr/lib/fglrx/dri/fglrx_dri.so
#11 0x00007f6df3a2d49b in ?? () from /usr/lib/fglrx/dri/fglrx_dri.so
#12 0x00007f6e26389572 in glXMakeCurrentReadSGI () from /usr/lib/fglrx/libGL.so.1
#13 0x00007f6e263898c1 in glXMakeCurrent () from /usr/lib/fglrx/libGL.so.1
#14 0x00007f6e238b7eb0 in QGLContext::makeCurrent() () from /usr/lib/libQtOpenGL.so.4
#15 0x00007f6e238723b6 in QGLPaintDevice::beginPaint() () from /usr/lib/libQtOpenGL.so.4
#16 0x00007f6e23872406 in ?? () from /usr/lib/libQtOpenGL.so.4
#17 0x00007f6e238a987d in QGL2PaintEngineEx::begin(QPaintDevice*) () from /usr/lib/libQtOpenGL.so.4
#18 0x00007f6e1fff7fd0 in QPainter::begin(QPaintDevice*) () from /usr/lib/libQtGui.so.4
#19 0x00007f6e1fff86f8 in QPainter::QPainter(QPaintDevice*) () from /usr/lib/libQtGui.so.4
#20 0x00007f6e257247dc in KisOpenGLCanvas2::paintEvent (this=0x582dce0) at /home/sven/kde/src/koffice/krita/ui/opengl/kis_opengl_canvas2.cpp:121
#21 0x00007f6e1fef5af2 in QWidget::event(QEvent*) () from /usr/lib/libQtGui.so.4
#22 0x00007f6e25725c76 in KisOpenGLCanvas2::event (this=0x582dce0, e=0x7fff75236880) at /home/sven/kde/src/koffice/krita/ui/opengl/kis_opengl_canvas2.cpp:402
#23 0x00007f6e1fe9fc0c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQtGui.so.4
#24 0x00007f6e1fea60eb in QApplication::notify(QObject*, QEvent*) () from /usr/lib/libQtGui.so.4
#25 0x00007f6e20baea16 in KApplication::notify(QObject*, QEvent*) () from /usr/lib/libkdeui.so.5
#26 0x00007f6e25e05d9c in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/libQtCore.so.4
#27 0x00007f6e1fefe0ed in QWidgetPrivate::drawWidget(QPaintDevice*, QRegion const&, QPoint const&, int, QPainter*, QWidgetBackingStore*) () from /usr/lib/libQtGui.so.4
#28 0x00007f6e200b6ffc in QWidgetPrivate::repaint_sys(QRegion const&) () from /usr/lib/libQtGui.so.4
#29 0x00007f6e1feefa04 in QWidgetPrivate::syncBackingStore() () from /usr/lib/libQtGui.so.4
#30 0x00007f6e1fef61f5 in QWidget::event(QEvent*) () from /usr/lib/libQtGui.so.4
#31 0x00007f6e25725c76 in KisOpenGLCanvas2::event (this=0x582dce0, e=0x518fba0) at /home/sven/kde/src/koffice/krita/ui/opengl/kis_opengl_canvas2.cpp:402
#32 0x00007f6e1fe9fc0c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQtGui.so.4
#33 0x00007f6e1fea60eb in QApplication::notify(QObject*, QEvent*) () from /usr/lib/libQtGui.so.4
#34 0x00007f6e20baea16 in KApplication::notify(QObject*, QEvent*) () from /usr/lib/libkdeui.so.5
#35 0x00007f6e25e05d9c in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/libQtCore.so.4
#36 0x00007f6e25e09454 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/libQtCore.so.4
#37 0x00007f6e1ff50d94 in ?? () from /usr/lib/libQtGui.so.4
#38 0x00007f6e25e046c2 in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQtCore.so.4
#39 0x00007f6e25e04a9c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQtCore.so.4
#40 0x00007f6e25e0973b in QCoreApplication::exec() () from /usr/lib/libQtCore.so.4
#41 0x00007f6e2611f253 in kdemain (argc=1, argv=0x7fff752377d8) at /home/sven/kde/src/koffice/krita/main.cc:49
#42 0x00000000004009c6 in main (argc=1, argv=0x7fff752377d8) at /home/sven/kde/build/koffice/krita/krita_dummy.cpp:3

Reported using DrKonqi
Comment 1 Dmitry Kazakov 2010-10-26 19:21:37 UTC
Looks like a build problem, but i'll better check on a release build.
Comment 2 Kubuntiac 2010-10-30 09:15:26 UTC
Wasn't able to reproduce on my RelWithDebInfo build and  r1191226 + KDE 4.5.2 QT 4.7.0 GCC 4.4.4 ATI driver and 4 cores (Kubuntu Maverick 32 bit)
Comment 3 Halla Rempt 2010-11-06 10:47:21 UTC
I cannot reproduce it either. Were there more layers in the image? What was the size?
Comment 4 Sven Langkamp 2010-11-06 14:55:50 UTC
*** Bug 254394 has been marked as a duplicate of this bug. ***
Comment 5 LukasT 2010-11-11 17:50:18 UTC
I can't reproduce.
Comment 6 Sven Langkamp 2010-12-04 16:36:10 UTC
*** Bug 219503 has been marked as a duplicate of this bug. ***
Comment 7 Dmitry Kazakov 2010-12-13 22:25:01 UTC
	A	 krita/image/kis_safe_read_list.h	 [License: UNKNOWN]

commit 0a253ac8d265ac4bf5fdb805bf6ff7fba8d00170
Author: Dmitry Kazakov <dimula73@gmail.com>
Date:   Mon Dec 13 22:24:56 2010 +0300

    Fixed a really weird race condition in KisNode
    
    I was fighting with this bug about two weeks (in total). The bug was
    really nasty. And the most weird thing, it could be reproduced in case
    your cpu has three or more cores! I've managed to catch it only thanks
    to the 6+6 cores server I was given access to.
    
    Ok, now about a bug. It happened due to Qt's cool feature called
    "implicit-sharing" coupled with the const methods having the same name
    as non-const names. Ok. Imagine a single KisNode object having some
    childs in a QList m_d->nodes. Now imagine three threads fighting for
    the read access to this node:
    
    1Th, 2Th, 3Th -- threads
    LA - initial list contents before implicit-sharing copying done
    LB - final list contents after implicit-sharing copying has been done
    
    1Th: reads KisNodeSP &var1 = m_d->nodes.first(); // Notice the
                                                        reference
         //  var1 will be equal to LA.first();
    1Th: gets suspended
    
    2Th: starts foreach cycle in childNodes() method
         foreach creates an implicit, *shared*, const copy of the initial
         list (i'm not sure, why qt's decided to do so, anyway, they do
         not support thread-safety officially).
    
    2Th: gets suspended
    
    /* the funniest part */
    3Th: calls KisNodeSP &var3 = m_d->nodes.first();
    
         Do you really think var3 will be equal to LA.first()? He-he.. ;)
         After the call is done, m_d->nodes reincarnates into LB(!). So
         var3 will be equal to LB.first(). The reference to the LA is
         holded by foreach loop from now on.
    
    3Th: gets suspended
    
    /* the show is going on */
    2Th: wakes up...
         The foreach loop finishes. Now it is (officially) the only owner
         of LA... So the best choice is to delete LA completely.
    2Th: is done
    
    1Th: is waking up... crash...
    
    I've made up a solution for this by creating a KisSafeReadList. This
    is a simple QList but the access to it is hightly limited: only
    constant methods are allowed and no copy-constructor at all. So
    effectively, it desables implicit-sharing.
    
    Now it works, but now i should search for similar bugs in KisBaseNode
    and in KoProperties.
    
    BUG:254545
    CCMAIL:kimageshop@kde.org

diff --git a/krita/image/kis_node.cpp b/krita/image/kis_node.cpp
index 58996f1..5f27951 100644
--- a/krita/image/kis_node.cpp
+++ b/krita/image/kis_node.cpp
@@ -33,6 +33,9 @@
 #include "kis_node_visitor.h"
 #include "kis_node_progress_proxy.h"
 
+#include "kis_safe_read_list.h"
+typedef KisSafeReadList<KisNodeSP> KisSafeReadNodeList;
+
 
 /**
  * The link between KisProjection ans KisImageUpdater
@@ -57,7 +60,7 @@ public:
 
     KisNodeWSP parent;
     KisNodeGraphListener * graphListener;
-    QList<KisNodeSP> nodes;
+    KisSafeReadNodeList nodes;
     KisNodeProgressProxy* nodeProgressProxy;
 };
 
@@ -75,8 +78,10 @@ KisNode::KisNode(const KisNode & rhs)
 {
     m_d->parent = 0;
     m_d->graphListener = rhs.m_d->graphListener;
-    foreach(const KisNodeSP & node, rhs.m_d->nodes) {
-        KisNodeSP children = node.data()->clone();
+
+    KisSafeReadNodeList::const_iterator iter;
+    FOREACH_SAFE(iter, rhs.m_d->nodes) {
+        KisNodeSP children = (*iter)->clone();
         children->createNodeProgressProxy();
         m_d->nodes.append(children);
         children->setParent(this);
@@ -174,7 +179,7 @@ KisNodeSP KisNode::nextSibling() const
 
 quint32 KisNode::childCount() const
 {
-    return m_d->nodes.count();
+    return m_d->nodes.size();
 }
 
 
@@ -200,16 +205,24 @@ QList<KisNodeSP> KisNode::childNodes(const QStringList & nodeTypes, const KoProp
 {
     QList<KisNodeSP> nodes;
 
-    foreach(const KisNodeSP & node, m_d->nodes) {
-        if (!nodeTypes.isEmpty()) {
-            foreach(const QString & nodeType,  nodeTypes) {
-                if (node->inherits(nodeType.toAscii())) {
-                    if (properties.isEmpty() || node->check(properties))
-                        nodes.append(node);
+    KisSafeReadNodeList::const_iterator iter;
+    FOREACH_SAFE(iter, m_d->nodes) {
+        if (properties.isEmpty() || (*iter)->check(properties)) {
+            bool rightType = true;
+
+            if(!nodeTypes.isEmpty()) {
+                rightType = false;
+                foreach(const QString &nodeType,  nodeTypes) {
+                    if ((*iter)->inherits(nodeType.toAscii())) {
+                        rightType = true;
+                        break;
+                    }
                 }
             }
-        } else if (properties.isEmpty() || node->check(properties))
-            nodes.append(node);
+            if(rightType) {
+                nodes.append(*iter);
+            }
+        }
     }
     return nodes;
 }
@@ -278,12 +291,7 @@ bool KisNode::remove(quint32 index)
 
 bool KisNode::remove(KisNodeSP node)
 {
-    if (node->parent().data() != this) {
-        return false;
-    }
-
-    return remove(index(node));
-
+    return node->parent().data() == this ? remove(index(node)) : false;
 }
 
 KisNodeProgressProxy* KisNode::nodeProgressProxy() const
diff --git a/krita/image/kis_node.h b/krita/image/kis_node.h
index 16e9017..c76b650 100644
--- a/krita/image/kis_node.h
+++ b/krita/image/kis_node.h
@@ -33,6 +33,11 @@ class KisNodeProgressProxy;
  * A KisNode is a KisBaseNode that knows about its direct peers, parent
  * and children and whether it can have children.
  *
+ * THREAD-SAFETY: All const methods of this class and setDirty calls
+ *                are considered to be thread-safe(!). All the others
+ *                especially add(), remove() and setParent() must be
+ *                protected externally.
+ *
  * NOTE: your subclasses must have the Q_OBJECT declaration, even if
  * you do not define new signals or slots.
  */
diff --git a/krita/image/kis_safe_read_list.h b/krita/image/kis_safe_read_list.h
new file mode 100644
index 0000000..c4af651
--- /dev/null
+++ b/krita/image/kis_safe_read_list.h
@@ -0,0 +1,98 @@
+/*
+ *  Copyright (c) 2010 Dmitry Kazakov <dimula73@gmail.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef KIS_SAFE_READ_LIST_H_
+#define KIS_SAFE_READ_LIST_H_
+
+
+#include <QList>
+
+/**
+ * \class KisSafeReadList
+ *
+ * This is a special wrapper around QList class
+ * Q: Why is it needed?
+ * A: It guarantees thread-safety of all the read requests to the list.
+ *    There is absolutely *no* guarantees for write requests though.
+ * Q: Why pure QList cannot guarantee it?
+ * A: First, Qt does not guarantee thread-safety for QList at all.
+ *    Second, QList is implicitly shared structure, therefore even
+ *    with read, but non-const requests (e.g. non-const QList::first()),
+ *    QList will perform internal write operations. That will lead to
+ *    a race condition in an environment with 3 and more threads.
+ */
+template<class T> class KisSafeReadList : private QList<T> {
+public:
+    KisSafeReadList() {}
+
+    using QList<T>::const_iterator;
+
+    /**
+     * All the methods of this class are splitted into two groups:
+     * treadsafe and non-threadsafe. The methods from the first group
+     * can be called concurrently with each other. The ones form
+     * the other group can't be called concurrently (even with the
+     * firends from the first group) and must have an exclusive
+     * access to the list.
+     */
+
+    /**
+     * The thread-safe group
+     */
+
+    inline const T& first() const {
+        return QList<T>::first();
+    }
+
+    inline const T& last() const {
+        return QList<T>::last();
+    }
+
+    inline const T& at(int i) const {
+        return QList<T>::at(i);
+    }
+
+    using QList<T>::constBegin;
+    using QList<T>::constEnd;
+    using QList<T>::isEmpty;
+    using QList<T>::size;
+    using QList<T>::indexOf;
+    using QList<T>::contains;
+
+    /**
+     * The non-thread-safe group
+     */
+
+    using QList<T>::append;
+    using QList<T>::prepend;
+    using QList<T>::insert;
+    using QList<T>::removeAt;
+    using QList<T>::clear;
+
+private:
+    Q_DISABLE_COPY(KisSafeReadList);
+};
+
+
+#define FOREACH_SAFE(_iter, _container)         \
+    for(_iter = _container.constBegin();        \
+        _iter != _container.constEnd();         \
+        _iter++)
+
+
+#endif /* KIS_SAFE_READ_LIST_H_ */
diff --git a/krita/image/tests/kis_node_test.cpp b/krita/image/tests/kis_node_test.cpp
index a0606f4..0a13057 100644
--- a/krita/image/tests/kis_node_test.cpp
+++ b/krita/image/tests/kis_node_test.cpp
@@ -370,6 +370,81 @@ void KisNodeTest::testDirtyRegion()
 #endif
 }
 
+#define NUM_CYCLES 100000
+#define NUM_THREADS 30
+
+class VisibilityKiller : public QRunnable {
+public:
+    VisibilityKiller(KisNodeSP victimNode, KisNodeSP nastyChild)
+        : m_victimNode(victimNode),
+          m_nastyChild(nastyChild)
+    {}
+
+    void run() {
+
+        int visibility = 0;
+
+        for(int i = 0; i < NUM_CYCLES; i++) {
+            if(i % 3 == 0) {
+                m_nastyChild->setVisible(visibility++ & 0x1);
+                // qDebug() << "visibility" << i << m_nastyChild->visible();
+            }
+            else if (i%3 == 1){
+                KoProperties props;
+                props.setProperty("visible", true);
+
+                QList<KisNodeSP> visibleNodes =
+                    m_victimNode->childNodes(QStringList("TestNodeB"), props);
+
+                foreach(KisNodeSP node, visibleNodes) {
+                    m_nastyChild->setVisible(visibility++ & 0x1);
+                }
+                // qDebug() << visibleNodes;
+            }
+            else {
+                Q_ASSERT(m_victimNode->firstChild());
+                Q_ASSERT(m_victimNode->lastChild());
+
+                m_victimNode->firstChild()->setVisible(visibility++ & 0x1);
+                m_victimNode->lastChild()->setVisible(visibility++ & 0x1);
+            }
+        }
+    }
+
+private:
+    KisNodeSP m_victimNode;
+    KisNodeSP m_nastyChild;
+};
+
+
+void KisNodeTest::propertiesStressTest() {
+    KisNodeSP root = new TestNodeA();
+
+    KisNodeSP a = new TestNodeA();
+    KisNodeSP b = new TestNodeB();
+    KisNodeSP c = new TestNodeC();
+    root->add(a, 0);
+    root->add(b, 0);
+    root->add(c, 0);
+    a->setVisible(true);
+    b->setVisible(true);
+    c->setVisible(true);
+    a->setUserLocked(true); 
+    b->setUserLocked(true);
+    c->setUserLocked(true);
+
+    QThreadPool threadPool;
+    threadPool.setMaxThreadCount(NUM_THREADS);
+
+    for(int i = 0; i< NUM_THREADS; i++) {
+        VisibilityKiller *killer = new VisibilityKiller(root, b);
+
+        threadPool.start(killer);
+    }
+
+    threadPool.waitForDone();
+}
+
 QTEST_KDEMAIN(KisNodeTest, NoGUI)
 #include "kis_node_test.moc"
 
diff --git a/krita/image/tests/kis_node_test.h b/krita/image/tests/kis_node_test.h
index c59b3be..b74cb7f 100644
--- a/krita/image/tests/kis_node_test.h
+++ b/krita/image/tests/kis_node_test.h
@@ -119,6 +119,8 @@ private slots:
     void testSetDirty();
     void testChildNodes();
     void testDirtyRegion();
+
+    void propertiesStressTest();
 };
 
 #endif
Comment 8 Cyrille Berger 2010-12-14 18:52:32 UTC
SVN commit 1206506 by berger:

Fix: race conditions that happen mostly on systems with more than four cores.

Backport of 0a253ac8d265ac4bf5fdb805bf6ff7fba8d00170

Patch by dmitry, tested by Sven.

CCBUG: 254545


 M  +24 -16    kis_node.cpp  
 M  +5 -0      kis_node.h  
 M  +75 -0     tests/kis_node_test.cpp  
 M  +2 -0      tests/kis_node_test.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1206506