mltframework
Producer creation using xml is not thread-safe and can lead to double free
This happens because the xml-producer has side-effects on the profile. Hence, if several clips are concurrently created using the same profile, nasty things can happen.
One such example is producer_xml.c:348. The profile description is freed then reallocated on the next line, through a strdup. If two threads execute this line at the same time, a double-free occurs. Regardless of that though, concurrent writes can lead to unexpected behaviour.
I couldn't produce a minimal code that hits a double free, but it does happen in prod.
With Flowblade an unreproducible " `python': double free or corruption" is the most common unexplained crash that happens when I do edits, and doing edits that require creating multiple new producers like when cutting clips on multiple tracks seem to be more vulnerable for this to occur.
This pattern of crashes supports this theory of a problem with non-thread safe producer creation causing double frees.
No, so it is not this Issue exactly. I will look through all the free()'s after next release to see if there are more of the same pattern as in producer_xml.c:348.
This just seemed similar to my longtime issues. These happen rarely and cannot be reproduced easily, so maybe we should just make all free()'s that could possibly be hit twice by different threads thread safe somehow.
Ok, that's indeed warrants further investigation.
Regarding the xml producer, it is counter-intuitive that creating the producer has side-effects on the profile. This should be either prevented, preventable or at least documented.
The XML DTD, which is referenced at the top of the documentation, defines a profile attribute and element. What did you think those would do? I think the application needs to prevent multi-threaded usage of the xml producer as the framework does not use threads to load producers.
In Kdenlive, we used the XML producer (in the same ways as Shotcut), for example to create a clip duplicate. We pass a producer through an xml consumer, then do something like: Mlt::Producer *prod = new Mlt::Producer(*myProfile, "xml-string", xml.toUtf8().constData()));
What surprised me is that the "profile" tag embedded in the xml does modify the "myProfile" passed as argument, so each use of "xml-string" producer does update all *myProfile properties. I somehow assumed that if the "*myProfile" was explicit, it would be prioritized over the "profile" tag in xml.
I was thinking along the same lines when reflecting on how Shotcut does often serialize and deserialize the profile and thinking maybe I should add a no_profile property to the xml consumer.
I need to analyze more about the impact of changing the xml producer to not modify the profile if it is set explicit because, after all, the composition may be based on its profile (when using frame numbers and pixel-based position and size values). Another factor is that, when the profile is explicit, the loader producer may load the xml producer with the consumer producer to interpret the composition within its own profile.
I have done all I am going to do in the near term on this in the above commits. I think another thing that can be done is to add lock and unlock functions on mlt_profile, but the application may also need to cooperate on that depending on what it does with mlt_profile. Shall I leave this open?