AviSynth / AviSynthPlus

AviSynth with improvements
http://avs-plus.net
971 stars 75 forks source link

Memory leak in 3.6.0 #178

Open chainikdn opened 4 years ago

chainikdn commented 4 years ago

How to reproduce: take MPC-HC (x64 to be specific) + ffdshow raw filter. Start playback, open ffdshow properties, put something memory-consuming into Avisynth script, e.g.

BicubicResize(5000,3000)
BicubicResize(1920,1080)

then check / uncheck Avisynth several times and observe memory usage via Task Manager.

3.6.0, 3.6.1: memory usage is rising with every check/uncheck cycle 3.5.1: memory usage is stable


I must say that the exact same issue were here years ago, but magically disappeared at some point after pinterf started its hard work.


Moreover, 3.6.1 crashes a lot. The simple way to reproduce is if you have SVP installed - then MPC-HC + AVS 3.6.1 crashes on every seek (after script reloading via on/off). Something is really wrong in 3.6 with cache or MT or both.

aportale commented 4 years ago

I can reproduce the memory consumption issue. image

qyot27 commented 4 years ago

ffdshow has not been updated in years, and as memory serves, the 64-bit version was known for having issues.

Given the changes in 3.6 that also change how client programs using its API directly need to work, it doesn't surprise me that something could potentially cause ffdshow to freak out, although I'm reasonably sure it never used any of the unstable IScriptEnvironment2 API stuff that were what caused other programs and plugins to complain (the point, there, is that those programs/plugins had ignored the warnings that that API was unstable and subject to change and future breakage; it took 4+ years for it to happen, but with 3.6, it did).

Memory leaks are also kind of difficult to find. Especially on Windows, given the (un)willingness - on my part, anyway - to want to mess with MSVC's tools beyond the compiler itself. If the behavior can be replicated on Linux, where it can be debugged under a more fully controllable environment (be it through gdb, or Clang's analysis tools or sanitizers, or GCC's sanitizers) and the actual piece of the source responsible for the problem can be found, then there's a much better chance of resolving the matter. Of course, that relies on having something that actively refreshes an AviSynth script during playback, rather than fully unloading the entire script environment and the demuxer, the way libavformat does.

A robust way of handling this would, aside from setting up CI builds using the sanitizers¹, actually having some sort of unit testing framework that can verify whether changes to the behavior of functions or API introduce regressions (either like this or as incorrect output, assuming the cases used have deterministic outcomes). I believe unit testing was on the suggested projects page on the website, for the attempt at getting into GSoC several years back, but neither of those things ended up happening. And that particular page wasn't hosted on the gh-pages branch.

¹maybe even -Wall, too. Not every warning is necessarily critical to fix, but there's always the possibility some mixture of things we get warned about might interact in a way that lies at the heart of other issues people have.

pinterf commented 4 years ago

Resizers: Memory leak would mean that deleting of IScriptEnvironment does not occur properly. Resizers use memory buffer pool internally, possibly its free-up is not done.

3.6.1 crash on seek: Surprisingly crashing is a bit more debug friendly thing as long as the crash frequently happens which is the case of your 2nd report.

(I'm back from holiday but after returning I will have a lot of accumulated office work, things will proceed a bit slower but I'm alive)

pinterf commented 4 years ago

Interesting project. Yet another museum section :) Anyway, try this ffdshow build and tell me what happens. For more info, see readme.txt in the zip. https://drive.google.com/file/d/1HAjqkbSALy3IHi3NI3QZn6BkzU_tZzPs/view?usp=sharing

pinterf commented 4 years ago

As for crashes, I have found this comment in the source: https://github.com/AviSynth/AviSynthPlus/blob/master/avs_core/core/avisynth.cpp#L1123

chainikdn commented 4 years ago

I have found this comment in the source

Are there different IScriptEnvironment's for each thread? oO /me-feel-stupid

pinterf commented 4 years ago

Of course, it's not only a jump-table. They have separate TLS, local variable set, buffers. http://avisynth.nl/index.php/Avisynthplus/Developers "Do not store the IScriptEnvironment pointer anywhere yourself (except locally on the stack), and never reuse those pointers outside of the methods where they were supplied to you. Not even between different executions of the same method! There is a reason why you get that pointer separately for each method, which is that it may be different every time, especially in multithreaded scenarios. If you reuse it, the consequences will be different between every implementation, but you can get anything from race conditions to program crashes."

chainikdn commented 4 years ago

... while working on fixing this one - 'svpflow ... ignores env given for GetFrame, and always use main thread's env' - I'm facing the issue that GetFrame is sometimes called more than once for the given frame. How is it possible? I was under impression that all subsequent requests must be cached. Happens right after the initialization (first ~10-20 frames), and the following situation is possible:

pinterf commented 4 years ago

Once the frame is in the cache it will return that. But I can imagine a timing scenario when the cache is slowly filled up by the requests of prefetcher while a filter is requesting the Nth frame immediately. There is no such cache state that a frame is "almost ready". So there is no wait and blocking.

pinterf commented 4 years ago

You can experiment with 3.6.x (Neo) modes: SetCacheMode(0) or SetCacheMode(CACHE_FAST_START) start up time and size balanced mode SetCacheMode(1) or SetCacheMode(CACHE_OPTIMAL_SIZE) slow start up but optimal speed and cache size

chainikdn commented 4 years ago

Anyway, try this ffdshow build and tell me what happens.

Yeah, it seems to free memory correctly. But as nobody will ever maintain ffdshow again :), is it possible to add (revert?) a workaround into AVS?


Not so sure making GetFrame use its own env changed anything o_O Too many combinations now to check - AVS 3,5 or 3.6, ffdshow "official" or patched, svpflow libs fixed for correct env usage or not. Plus the crashes are not always easily reproducible. /signs...

pinterf commented 4 years ago

I don't think there can be an avs workaround. ffdshow is not freeing up the ScriptEnvironment. It seems that older avisynth versions tolerated it somehow. Plus I made another fix in this test ffdshow to prevent random crashes (seen on x64), more frequently in debug build than in release mode.

aportale commented 4 years ago

Interesting project. Yet another museum section :)

I used ffdshow much a couple of years ago. Great project, sadly abandoned. However, FFMS2 suits my needs nowadays and I am not really looking back. I just installed ffdshow after Years to reproduce this issue (and a bit for the nostalgia).

Anyway, try this ffdshow build and tell me what happens.

Yes, Your version avoids the leak!

qyot27 commented 4 years ago

A lot of things with media player selection have changed since ffdshow was actively developed. LAV Filters have supplanted it for strictly DirectShow playback, if the user doesn't just eschew it entirely and use VLC or mpv (which, considering ffdshow was essentially a port of mplayer to DShow, the filtering system in mpv would be more robust anyway).

https://github.com/CrendKing/avisynth_filter now exists to replicate the AviSynth integration that ffdshow had. As I never used that even back when I did bother with DShow-based players, I can't do any comparison on pros and cons there.

Really, the only thing ffdshow still has that modern solutions don't is the Video for Windows interface. Or more specifically, ffdshow's lack of development has meant that the VfW interface is using such an old version of the libav* libraries that it doesn't support FFV1 version 3 (amongst other various formats), which has popped up consistently as an issue. Working around it means disabling features that make FFV1 more efficient just so people can still use AVISource() with them.

Nuihc88 commented 4 years ago

This issue doesn't bother me personally, but if someone wants to take the time to track this leak down, here's a place to start:

The first build in my archives to contain this memory leak was r3132 from "AvisynthPlus-3.5.test_20200402b-filesonly.7z".

Both r3072 from "AvisynthPlus-3.5.test_20200322-filesonly.7z" & r3106 from "AviSynthPlus_3.5.1_20200402_filesonly.7z" are fine.