Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leaks caused by multitrack() and track() calls #1592

Closed
bmatherly opened this issue Oct 5, 2024 · 3 comments
Closed

Memory leaks caused by multitrack() and track() calls #1592

bmatherly opened this issue Oct 5, 2024 · 3 comments
Assignees
Milestone

Comments

@bmatherly
Copy link
Member

The following functions return "new" objects:

Mlt::Multitrack::Track()
https://github.com/mltframework/mlt/blob/c75721eaae09196cb523132d63f40efc3882eecd/src/mlt%2B%2B/MltMultitrack.cpp#L87

Mlt::Tractor::Multitrack()
https://github.com/mltframework/mlt/blob/c75721eaae09196cb523132d63f40efc3882eecd/src/mlt%2B%2B/MltTractor.cpp#L94

Mlt::Tractor::Track()
https://github.com/mltframework/mlt/blob/c75721eaae09196cb523132d63f40efc3882eecd/src/mlt%2B%2B/MltTractor.cpp#L124

There are multiple instances in Shotcut where we call these functions without deleting the returned objects.

As an example, this line calls both Multitrack() and Track() without freeing either returned object. So two objects are being leaked.

Mlt::Producer producer(m_model.tractor()->multitrack()->track(mlt_index));

The purpose of this issue is to remind us to do a search through the code to seek out these leaks and clean them up.
Any volunteers who want to work on this should assign this to themselves to let others know that they are working on it.

@ddennedy ddennedy self-assigned this Oct 5, 2024
@ddennedy
Copy link
Member

ddennedy commented Oct 6, 2024

Qt Creator makes this task easy for me by searching for all references to a symbol. BTW it cannot find references using the old style connect(SIGNAL() .. SLOT()) methods, which is one reason why I try to avoid introducing them.

During this, it surfaced some usage of bare pointers that I replaced. It is easier to locate these locates by looking for results without QScopedPointer or std::unique_ptr.

@bmatherly
Copy link
Member Author

BTW it cannot find references using the old style connect(SIGNAL() .. SLOT())

I agree - I always try to use the new style as well. Do we have any interest in a clean-up task to convert the remaining instance of old connect statements? Maybe the risk is not worth the reward.

@ddennedy ddennedy added this to the v24.10 milestone Oct 6, 2024
@ddennedy
Copy link
Member

ddennedy commented Oct 6, 2024

I think it can be worth it to convert connect, but one must be rather careful because you need to specify the correct class name, which is not always obvious with method overrides. When you get it wrong there can be behavior differences from the SIGNAL or SLOT approach. That's one reason why I have held off besides the fact that there are many of these non-critical code maintenance tasks one could do such as modern C++ replacements to Qt features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants