On Tue, Jan 12, 2010 at 7:30 AM, D. Michael McIntyre <michael.mcint...@rosegardenmusic.com> wrote: > The pointer is still pointing at 12345678 in the dtor, and the only other code > I see that could possibly delete the device is in the accept() method, which > is not firing.
The device that is pushed into m_devices at line 379 does not directly "belong" to the dialog. The device pointer just points to a device within m_fileDoc's studio (note the earlier call to m_fileDoc->getStudio().getDevices(), which just returns a pointer to the studio's internal device list). m_fileDoc is a temporary document, which is deleted from the dialog's destructor before the line in which the crash happens; the document's destructor deletes the studio and the devices in it; therefore the crash happens because of a double deletion. The obvious solution is not to delete the devices, neither at the line where this crash occurs nor in accept(). Instead accept() should just clear the device list without deleting the devices, while copying the chosen device to m_device. The destructor is clever enough not to delete m_device if the temporary document exists, correctly deleting the document instead. So long as the code that calls this dialog is not able to refer to its m_device after the dialog has been deleted (which is the case, because the dialog has no accessor that returns m_device -- any lookups on the device have to happen before deleting the dialog), this should work safely, and should not leak. An alternative solution would be to duplicate the devices when pushing them to m_devices at line 379 (i.e. m_devices.push_back(new MidiDevice(*device))). And in case you're thinking "woo, nice explanation, he's clever" -- I introduced this bug myself, in revision 11047. Check the diff for this file in that revision, it does absolutely nothing except introduce this bug. I must have thought there was a leak and not followed through the train of thought properly (nor, evidently, tested it). I'm sorry for wasting your time like that. (I would like to blame the original code, which I didn't write, for inviting mistakes like this by using such a stupid object ownership structure. Any time you find yourself having to decide whether or not to delete an object on the basis of whether or not another object, which might own it, still exists, you're almost certainly doing something wrong. But that's no excuse for my adding a new bug and failing to test it.) Chris ------------------------------------------------------------------------------ This SF.Net email is sponsored by the Verizon Developer Community Take advantage of Verizon's best-in-class app development support A streamlined, 14 day to market process makes app distribution fast and easy Join now and get one step closer to millions of Verizon customers http://p.sf.net/sfu/verizon-dev2dev _______________________________________________ Rosegarden-devel mailing list Rosegarden-devel@lists.sourceforge.net - use the link below to unsubscribe https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
