KaxBlockBlob brokenness
Dennis Filder via Matroska-devel <[email protected]> Sat, 24 Apr 2021 16:45:49 +0200
| Newsgroups | gmane.comp.multimedia.matroska.devel |
|---|---|
| Message-ID | <20210424144549.GA3567@reader> |
Hi, in my on-going quest to get the test suite working again I've hit a bit of a roadblock. The way I see it the class KaxBlockBlob in its current form is inherently broken due to its member "Block". If used as intended the .group/.simpleblock points at a KaxBlockGroup or KaxSimpleBlock instance. The problem is that this instance will sooner or later end up as a child in a KaxCluster instance whose ~EbmlMaster() dtor will then invoke "delete" on it. ~KaxBlockBlob() will try the same leading to a double-free and segfault. To make matters worse, KaxReferenceBlock::SetReferencedBlock(const KaxBlockGroup &) allocates a KaxBlockBlob instance and hooks the passed-in KaxBlockGroup (which stems from a previously written block and is now reused as a B- or P-frame referent) to it and then sets bOurBlob=true. The call to FreeBlob() (introduced with commit 697ce733589c44ac10dd46d46bfd28a075dea5cc) in ~KaxReferenceBlock() then frees it, but in the mean time, the dtor of the referent's parent (a KaxCluster) could have already freed it leading to another double-free and segfault. I could get mux/test6.cpp to run only by disabling any deletes on KaxBlockBlob::Block.group, but that of course is not a permanent remedy. How is this supposed to work/be fixed? I was thinking up thousands of clever little additions to solve this with bools, but my conclusion is that the only way to not make this even worse is to involve shared_ptr. One approach could be to involve shared_ptr only for those elements which have another element point to it. This could work by attaching to the Segment instance a map that maps raw pointer values to shared_ptr instances. When associating a KaxBlockBlob with a KaxBlockGroup or KaxSimpleBlock (or creating any other kind of thing referencing another element), a shared_ptr is created from the raw pointer and a copy of it is put into the map for ~EbmlMaster() to free. ~EbmlMaster() could then (1) walk up the tree (for which it would probably need a schema-specific lambda since EbmlElement has no GetParent()) to get to the EbmlMaster holding the map, (2) then look up each raw pointer from Element_List in it, and (3) if nothing is found invoke "delete" on the raw pointer, otherwise call reset() on the retrieved shared_ptr. KaxBlockBlob::Block would have to become a union of shared_ptr, of course. This would clarify the ownership rules in the library by mandating that an object is owned by its parent EbmlMaster unless it has been entered into the Segment's map in which case shared_ptr rules apply. Let me know what you think of this (and also why EbmlElement lacks pointers to the parent element -- given that it would have been trivial to add it must have been omitted for a good reason). Regards, Dennis. _______________________________________________ Matroska-devel mailing list [email protected] https://lists.matroska.org/cgi-bin/mailman/listinfo/matroska-devel Read Matroska-Devel on GMane: http://dir.gmane.org/gmane.comp.multimedia.matroska.devel