<div dir="ltr"><div><span style="font-size:13px">> Well, I cannot agree with it. Passing a raw pointer down the stack and acquiring it somewhere below will make the code crashy and really hard to understand why. Who would guarantee how the > passed object will be used inside the function? Will it be stored or not? How the caller has allocate and stored the pointer? Does he own it or not? There might be really funny situations, like:</span><br></div><div><span style="font-size:13px"><br></span></div>I understand the issue you present and i think there's a difference on what "we want" on a codebase:<div>I normally want to be in control of the codebase and know it very well so if something like that happens i would know why it happened.</div><div><br></div><div><div>Don't want to go offtopic but "the developer shouldn't care" it's more fitted for a library, but library are quite hard to make, so that the developer really doesn't care how the underlying code works (and still some information is given by the doc).</div><div>So what i mean is that i "never" trust, unless it's a well tested library, the functions i call etc and check what they actually do, so that i can also use them properly in other cases.</div></div><div><br></div><div>Again i understand the different point of view you may have, and respect it, but hiding too much, while it can help with information overhead, might also make actually harder to write code, imho.</div><div><br></div><div>Though when i said "pass down the stack a raw pointer", i meant in the case you already had a shared pointer holding it (like in the rootLayer() function in KisImage).</div><div>In the issue you present that memory should've been acquired by a shared pointer, especially since it's a class (this should actually be a must) and manual deletion should be banned for those types.</div><div><br></div><div>In general i think the same as him -> <a href="http://www.stroustrup.com/C++11FAQ.html#std-shared_ptr" target="_blank">http://www.stroustrup.com/C++11FAQ.html#std-shared_ptr</a>, not only because he his who he his, but because it actually make sense what he says about making the object lifetime cycle complex when shared pointer are used so often.</div><div><br></div><div>Maybe having unique_ptr like pointers and shared pointer passed as const references instead of raw pointer would be better, so that it's more explicit the fact that you should not delete it manually and it's cheaper, copy wise and with the ref/deref not happening when passed around in code that shouldn't actually prolong its life.</div><div>You would still have shared pointers around but it would be much easier for the memory leak tracker to keep track on who is really holding the memory.</div><div><br></div><div>> <span style="font-size:13px">The idea provided by Qt's shared pointers might be very helpful here (Qt's weak shared pointer doesn't allow dereferencing the pointer until upgrade to a real shared pointer)</span></div><div><span style="font-size:13px"><br></span></div><div><span style="font-size:13px">There are a couple of problems with Krita weak pointers i'm facing though:</span></div><div><span style="font-size:13px">1) If you pass a dead weak pointer to a shared pointer the check to see if the weak pointer is valid is done in an assert only after the shared pointers tries to increase the reference count.</span></div><div>This works if the weak pointer internal pointer was set to NULL, because shared pointer ref() function checks for that, but if the weak pointer points to deleted memory.. well ref() behaviour is undefined.</div><div><br></div><div>2) Here i'm talking about that issue with the resource manager holding a shared pointer to the current activated node, that should be deleted when the image is deleted.</div><div>You suggested to use a weak pointer but due to how QVariant works, as of now, you can't safely store (or actually retrieve) a weak pointer inside because there's no way to check isValid() function before a copy happens (through QVariant value<>() function), and this as point 1, may access to deleted memory.</div><div>This is kind of complicated because you can put the isValid check in the copy constructor before loading the pointer into the new weak shared pointer, so that the internal pointer is set to 0 if it's not valid, but it would add the overhead of the isValid check for a specific case to every copy done.</div><div><br></div><div>This is why i think sometimes smart pointers get a bit in the way, they make you think everything is safer while.. it depends.. </div><div>For point 2 i think then it should be who actually set that node (KoCanvasResourceProvider through an event), that always through an event (image closed/deletion?) removes it (so the QVariant too), instead of relying on a weak pointer?</div><div><br></div><div><span style="font-size:13px">> So as a conclusion for my long mail I would suggest to consider the following:</span><br></div><div><span style="font-size:13px"><br></span></div><div><span style="font-size:13px">Don't worry, as you can see mine is longer, obviously :-).</span></div><div><span style="font-size:13px"><br></span></div><div><div style="font-size:13px">> 1) Use weak shared pointers in UI dockers or elements.<br></div><div style="font-size:13px">> 2) Upgrade weak shared pointer to real pointer before use in UI (?)<br></div><div style="font-size:13px">> 3) Don't use manual deletion of Private classes. Use QScopedPointer<Private> instead. This is the most popular cause of memory leaks and dangling shared pointer in Krita, btw. It also guarantees the "c++ rule of three" is satisfied properly (it disables default copy-ctor by default).</div></div><div style="font-size:13px"><br></div><div style="font-size:13px">I agree with these, but can you elaborate more point 2 (what you mean with real pointer?).</div></div>