[Question] Memory concerns?

[Zulu-Inuoe]

Hi all, thanks for the great library.

I am trying to diagnose what appears to be a memory leak in one of our applications, and I am wanting to make sure I am not misusing or overlooking something with Handlebars. After running our application, which will dynamically load up Handlebars templates, for a long time, generating many templates, I see memory usage continue to grow.

There's many other pieces to this, so I didn't even consider Handlebars until I used Visual Studio's memory profiler and found this:

Where we have (this is a diff) over 200 MB of these ReaderWriterLockSlim objects, and they're all referenced from HandlebarsDotNet objects.

Our usage of Handlebars is as follows:

string OnEachRequest(...)
{
    var handlebars = Handlebars.Create();

    //Register custom helpers
    ...

    return handlebars.Compile(someTemplate)(someData);
}

Is there some sort of cleanup I'm forgetting to do?

Thanks! I will continue to dig into this further but figure it doesn't hurt to ask in case I'm missing something obvious.

[oformaniuk]

Hello @Zulu-Inuoe Thanks for your report. There's a chance that bug is hiding somewhere there as such memory consumption is not expected. I'd try to have a look once I have some time. Meanwhile any help with investigation is welcome.

[Zulu-Inuoe]

Update - The code involved actually doesn't use HandlebarsHelpers (the library). I've updated the description to match so I can rule that out.

[Zulu-Inuoe]

Also I'm tracing through the Handlebars code now (Had to take a break last night) and it looks like the bulk of the issue is traced back to the Lazy in Handlebars.cs I haven't been able to reproduce locally on my machine by just spinning a bunch of threads, though:

[Zulu-Inuoe]

Aha! I've found the problem. I was looking at the wrong code on our side - I switched my tests to use the global handlebars environment and am easily able to destroy my RAM.

My solution is going to be to switch from using Handlebars.Compile into using a fresh context each time. This is incorrect legacy code any way - the handlers that are registered are closing over some local variables, so registering in the global environment would lead to sporadic errors in a multi-threaded environment anyway

[Zulu-Inuoe]

@zjklee I think this should be marked as a bug. Running Handlebars 2.0.7, the following eats up all my RAM:

            while(true)
            {
                Handlebars.Compile("Hello, world!");
            }

I haven't dug down exactly what's happening but each call to Compile ends up throwing more things into that ObservableList that is ultimately referenced by the HandlebarsEnvironment held in that static variable, creating a memory leak.

I know most people (should) be using a fresh environment, but this is definitely a regression from v1.x

[oformaniuk]

I had some initial look and have some comments/questions/explanations. @Zulu-Inuoe , assuming the usage pattern mentioned above I'm not surprised that you see increased memory usage as each Compile creates new template object with lost of service object including various caches to optimize execution of the template.

However examining the code I can see where memory leak occurs. Looks like leak occurs due to not releasing (removing) Observers of HandlebarsConfiguration created for each template object. This can be fixed by replacing List<> by an implementation of WeakList<> inside of Observable* collections.

P.S. Using Handlebars.Compile should still be OK. You where not able to reproduce while using new instance of environment each time because GC was able to collect such leaked observers together with the whole environment.

[Zulu-Inuoe]

Yes, I agree with your observations. In our code in particular we should have been using fresh Handlebars environments anyway so the switch is fine.

And I get that each Compile allocs a new object, that's perfectly reasonable. The problem (and I believe you agree) is that as a byproduct it also allocates unreclaimable memory in the form of those observers you mentioned.

I appreciate you looking into this futher

[abraham-fox]

Hey, zjklee. Thanks for great work! Looks like the issue was fixed, but the pull request with fixes hasn't been merged yet. Do you have any ETA? We are looking forward to use the 2.0.* version at production.

[oformaniuk]

Hello @abraham-fox I had not time to verify whether the fix actually works (aka profile memory) and this is the reason it's still not merged. I should have few hours next week and hope to get it done. However, in case you can help to verify it I'd be able to merge it once I get confirmation.

[alexander-narbut]

We updated the package from version 1.9.5 to 2.0.8 and faced with OutOfMemoryException on production. Then reverted back. This is the top priority issue.

[Zulu-Inuoe]

@alexander-narbut In the interim as a solution you may change all instances of

Handlebars.Compile(...);

into

var handlebars = Handlebars.Create();
handlebars.Compile(...);

or even

Handlebars.Create().Compile(...);

as a one-liner.

This should work fine even if you are using global custom helpers, since each context falls back to the global one.

Edit: If you are using custom helpers you'll need to register them on this new Handlebars object

[oformaniuk]

@Zulu-Inuoe

This should work fine even if you are using global custom helpers, since each context falls back to the global one.

I'm not sure about this statement (at least as far as I remember codebase). Have you tried this on your own?

[oformaniuk]

@alexander-narbut Help with fix verification would definitely expedite the process.

[Zulu-Inuoe]

@zjklee Apologies - I just tested this and you are totally correct. I was misremembering the codebase