Repository navigation
Exponentially increasing amount of calls to TypeModule.CreateTypesAsync() after each OnTypesChanged() #9901
Description
Activity
Thanks for reporting this :) We'll have a look.
Suspected issue is an event-subscription leak in HotChocolate’s schema refresh path.
When
DummyTypeModule.RefreshTypes()callsOnTypesChanged(), HotChocolate starts rebuilding the request executor. During that rebuild it creates a newTypeModuleChangeMonitorand immediately subscribes it toDummyTypeModule.TypesChanged.If the rebuild then fails, for example because a reused
DateTimeTypeinstance causes schema initialization to throw, HotChocolate does not dispose that new monitor. So itsTypesChangedevent handler stays attached.On the next
RefreshTypes()call, both the real/current monitor and the leaked monitor reacts, causing multiple rebuild attempts. Each failed attempt leaks another handler, so the number ofCreateTypesAsync()calls keeps growing, often doubling.There are two separate things here.
The first is the bug you identified: when a schema rebuild fails, the type module change monitor is not disposed and its event subscription leaks, which is what causes the exponential growth in
CreateTypesAsync()calls. This is fixed with #9902.The second is the type registration itself. What you found is not a workaround, it's the required approach for dynamic schemas. A type instance registered via
.AddType(new Foo())is initialized into the first schema and bound to it; it cannot be initialized a second time. That's why your rebuilds were failing in the first place. Even with the leak fixed, instance registration would still prevent your schema from ever updating, so for schemas that rebuild at runtime, use.AddType(() => new Foo())or.AddType<Foo>()so each rebuild gets a fresh instance.The second is the type registration itself. What you found is not a workaround, it's the required approach for dynamic schemas. A type instance registered via
.AddType(new Foo())is initialized into the first schema and bound to it; it cannot be initialized a second time. That's why your rebuilds were failing in the first place. Even with the leak fixed, instance registration would still prevent your schema from ever updating, so for schemas that rebuild at runtime, use.AddType(() => new Foo())or.AddType<Foo>()so each rebuild gets a fresh instance.OK, thanks for the explanation. This seems like a footgun that could be avoided by always requiring a factory method.
Agreed. The TypeModule use case is a pretty uncommon one though and making the factory required would be a breaking change for everyone else, so I don't think we'll be changing it any time soon.
Product
Hot Chocolate
Version
16.1.3
Link to minimal reproduction
https://github.com/hroi/hotchoc-notifier-leak-repro
Steps to reproduce
TypeModule.AddType(new Foo())OnTypesChanged()What is expected?
A single
CreateTypesAsync()callback perOnTypesChangedcall.What is actually happening?
Exponentially increasing amount of calls to
CreateTypesAsync()between each call toOnTypesChanged()call.Relevant log output
info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 1 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 1 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 1 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 2 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 4 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 8 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 16 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 32 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 64 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 128 info: TypeModuleRepro.DummyTypeModule[0] CreateTypesAsync() calls since last RefreshTypes() call: 256Additional context
The workaround I found is to use a factory lambda instead:
.AddType(() => new Foo()).