-
Notifications
You must be signed in to change notification settings - Fork 235
WeakReference garbage collection #604
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
base: master
Are you sure you want to change the base?
WeakReference garbage collection #604
Conversation
| if (_comparer.Equals(key, entries[i].Key)) | ||
| { | ||
| entries[i].Value = value; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public bool Return(BindingContext item) | ||
| { | ||
| item.Configuration = null; | ||
|
|
||
| item.Root = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BindingContextPool may temporary prevent garbage collection of some configuration objects because the Configuration property isn't cleared when BindingContext is being returned to the pool. It's unlikely that this issue will cause actual problems because the pool will eventually cycle through all BindingContext objects and it will release the configuration object by assigning a new configuration object on:
| context.Configuration = configuration; |
But it makes sense to clear Configuration property when returning BindingContext to the pool because this property will be set to a new value in the CreateContext() method. It's better to clear it explicitly as not to prevent GC.
|
Thanks for working on this, I’m traveling but will keep an eye on it! |
| public void Add(T value) | ||
| { | ||
| // Need a way to reset _firstAvailableIndex periodically | ||
|
|
||
| for (var index = _firstAvailableIndex; index < _store.Count; index++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this spot is the main source of constant growth of GC Handlers and WeakReferences. I don't have a good idea about how to fix it and I'd appreciate some guidance.
This issue can only be observed if a static method Handlebars.Compile(source) is used, or if a Handlebars object is stored in a static variable (e.g. private static IHandlebars _handlebars = Handlebars.Create();), or HandlebarsConfiguration object is stored in a static variable.
Every time when a template is compiled, new instances of FormatterProvider and ObjectDescriptorFactory are created on:
Handlebars.Net/source/Handlebars/HandlebarsEnvironment.cs
Lines 132 to 133 in 9cf1672
| var formatterProvider = new FormatterProvider(configuration.FormatterProviders); | |
| var objectDescriptorFactory = new ObjectDescriptorFactory(configuration.ObjectDescriptorProviders); |
Those new instances are then subscribed to the original FormatterProvider and ObjectDescriptorFactory objects from the main configuration object.
WeakCollection has a mechanism to detect dead references and re-use WeakReference objects to store new values. It's done by resetting _firstAvailableIndex in Remove() and GetEnumerator(). But, if those two methods are never called, List<WeakReference<T> grows continuously.
ObservableIndex and ObservableList that utilize WeakCollection have Subscribe() methods which return a disposable container which calls Remove() when disposed.
Handlebars.Net/source/Handlebars/Collections/ObservableIndex.cs
Lines 34 to 52 in 9cf1672
| public IDisposable Subscribe(IObserver<ObservableEvent<TValue>> observer) | |
| { | |
| using (_observersLock.WriteLock()) | |
| { | |
| _observers.Add(observer); | |
| } | |
| var disposableContainer = new DisposableContainer<WeakCollection<IObserver<ObservableEvent<TValue>>>, ReaderWriterLockSlim>( | |
| _observers, _observersLock, (observers, @lock) => | |
| { | |
| using (@lock.WriteLock()) | |
| { | |
| observers.Remove(this); | |
| } | |
| } | |
| ); | |
| return disposableContainer; | |
| } |
But none of the methods that call Subscribe() save those disposable containers and therefore don't dispose them.
So maybe the fix should be to keep track of disposable containers in the methods that call Subscribe() and dispose them in finalizers?
@rexm thanks! I am still adding details to the PR and will hit "Ready for review" once done. |


While updating Handlebars.Net from 1.x to 2.1.6, I have noticed that GC Handler count increases significantly with every compiled template. GC Handlers are associated with WeakReference objects which were introduced in #456.
I discovered a couple of issues while investigating the GC Handlers increase.
My app complies templates using the static method
Handlebars.Compile().Before the Update
After the Update