ISession.Get may fail with a null exception

[RogerKratz]

ISession.Get fails with an "InvalidOperationException : Nullable object must have a value" when the entity class has batch lazy-loads enabled and at least one but less than batch-size other instances of the entity class are proxies awaiting loading.
(The get has to be done on an entity not yet loaded, even as an uninitialized proxy.)

Initial report:

Regression bug for 5.2.

We have a bunch of tests that after upgrade to 5.2 fails with stack trace below. Tried to figure out what/when this occur but no success yet. I guess there are some particular mapping that causes this?
It happens both in tests when Get as well as Merge is called.

The commit when this was added was 09120c0.

I've tried to isolate this problem to a small test but haven't really found out what causes this. ping @maca88 - do you have a guess when this could happen?

System.InvalidOperationException : Nullable object must have a value.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at NHibernate.Engine.BatchFetchQueue.<GetEntityBatch>g__CheckCacheAndProcessResult|25_0(<>c__DisplayClass25_0& )
   at NHibernate.Engine.BatchFetchQueue.GetEntityBatch(IEntityPersister persister, Object id, Int32 batchSize, Boolean checkCache)
   at NHibernate.Loader.Entity.BatchingEntityLoader.Load(Object id, Object optionalObject, ISessionImplementor session)
   at NHibernate.Persister.Entity.AbstractEntityPersister.Load(Object id, Object optionalObject, LockMode lockMode, ISessionImplementor session)
   at NHibernate.Event.Default.DefaultLoadEventListener.LoadFromDatasource(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options)
   at NHibernate.Event.Default.DefaultLoadEventListener.DoLoad(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options)
   at NHibernate.Event.Default.DefaultLoadEventListener.Load(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options)
   at NHibernate.Event.Default.DefaultLoadEventListener.ProxyOrLoad(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options)
   at NHibernate.Event.Default.DefaultLoadEventListener.OnLoad(LoadEvent event, LoadType loadType)
   at NHibernate.Impl.SessionImpl.FireLoad(LoadEvent event, LoadType loadType)
   at NHibernate.Impl.SessionImpl.InternalLoad(String entityName, Object id, Boolean eager, Boolean isNullable)
   at NHibernate.Type.EntityType.ResolveIdentifier(Object id, ISessionImplementor session)
   at NHibernate.Type.EntityType.Replace(Object original, Object target, ISessionImplementor session, Object owner, IDictionary copyCache)
   at NHibernate.Type.CollectionType.ReplaceElements(Object original, Object target, Object owner, IDictionary copyCache, ISessionImplementor session)
   at NHibernate.Type.CollectionType.Replace(Object original, Object target, ISessionImplementor session, Object owner, IDictionary copyCache)
   at NHibernate.Type.TypeHelper.Replace(Object[] original, Object[] target, IType[] types, ISessionImplementor session, Object owner, IDictionary copiedAlready)
   at NHibernate.Event.Default.DefaultMergeEventListener.CopyValues(IEntityPersister persister, Object entity, Object target, ISessionImplementor source, IDictionary copyCache)
   at NHibernate.Event.Default.DefaultMergeEventListener.EntityIsDetached(MergeEvent event, IDictionary copyCache)
   at NHibernate.Event.Default.DefaultMergeEventListener.OnMerge(MergeEvent event, IDictionary copiedAlready)
   at NHibernate.Impl.SessionImpl.FireMerge(IDictionary copiedAlready, MergeEvent event)
   at NHibernate.Impl.SessionImpl.Merge(String entityName, Object obj, IDictionary copiedAlready)
   at NHibernate.Engine.CascadingAction.MergeCascadingAction.Cascade(IEventSource session, Object child, String entityName, Object anything, Boolean isCascadeDeleteEnabled)
   at NHibernate.Engine.Cascade.CascadeToOne(Object parent, Object child, IType type, CascadeStyle style, Object anything, Boolean isCascadeDeleteEnabled)
   at NHibernate.Engine.Cascade.CascadeCollectionElements(Object parent, Object child, CollectionType collectionType, CascadeStyle style, IType elemType, Object anything, Boolean isCascadeDeleteEnabled)
   at NHibernate.Engine.Cascade.CascadeCollection(Object parent, Object child, CascadeStyle style, Object anything, CollectionType type)
   at NHibernate.Engine.Cascade.CascadeAssociation(Object parent, Object child, IType type, CascadeStyle style, Object anything, Boolean isCascadeDeleteEnabled)
   at NHibernate.Engine.Cascade.CascadeOn(IEntityPersister persister, Object parent, Object anything)
   at NHibernate.Event.Default.DefaultMergeEventListener.CascadeOnMerge(IEventSource source, IEntityPersister persister, Object entity, IDictionary copyCache)
   at NHibernate.Event.Default.DefaultMergeEventListener.EntityIsDetached(MergeEvent event, IDictionary copyCache)
   at NHibernate.Event.Default.DefaultMergeEventListener.OnMerge(MergeEvent event, IDictionary copiedAlready)
   at NHibernate.Impl.SessionImpl.FireMerge(IDictionary copiedAlready, MergeEvent event)
   at NHibernate.Impl.SessionImpl.Merge(String entityName, Object obj, IDictionary copiedAlready)
   at NHibernate.Engine.CascadingAction.MergeCascadingAction.Cascade(IEventSource session, Object child, String entityName, Object anything, Boolean isCascadeDeleteEnabled)
   at NHibernate.Engine.Cascade.CascadeToOne(Object parent, Object child, IType type, CascadeStyle style, Object anything, Boolean isCascadeDeleteEnabled)
   at NHibernate.Engine.Cascade.CascadeCollectionElements(Object parent, Object child, CollectionType collectionType, CascadeStyle style, IType elemType, Object anything, Boolean isCascadeDeleteEnabled)
   at NHibernate.Engine.Cascade.CascadeCollection(Object parent, Object child, CascadeStyle style, Object anything, CollectionType type)
   at NHibernate.Engine.Cascade.CascadeAssociation(Object parent, Object child, IType type, CascadeStyle style, Object anything, Boolean isCascadeDeleteEnabled)
   at NHibernate.Engine.Cascade.CascadeOn(IEntityPersister persister, Object parent, Object anything)
   at NHibernate.Event.Default.DefaultMergeEventListener.CascadeOnMerge(IEventSource source, IEntityPersister persister, Object entity, IDictionary copyCache)
   at NHibernate.Event.Default.DefaultMergeEventListener.EntityIsDetached(MergeEvent event, IDictionary copyCache)
   at NHibernate.Event.Default.DefaultMergeEventListener.OnMerge(MergeEvent event, IDictionary copiedAlready)
   at NHibernate.Event.Default.DefaultMergeEventListener.OnMerge(MergeEvent event)
   at NHibernate.Impl.SessionImpl.FireMerge(MergeEvent event)
   at NHibernate.Impl.SessionImpl.Merge(String entityName, Object obj)
   at NHibernate.Impl.SessionImpl.Merge[T](T entity)

[fredericDelaporte]

If you could run your tests with the pdb from the symbol package, it should then add which line of code has failed, which may help.

The symbol package is published on https://nuget.smbsrc.net, but currently it has a certificate trouble...
PDBs are also available in the SourceForge release (https://sourceforge.net/projects/nhibernate/files/NHibernate/5.2.0/, bin package) or directly in the TeamCity build artifacts (for v5.2.0, here).

[RogerKratz]

Ah, thanks for the tip about sourceforge (and TC build), had the same problem as you with nuget's package source.

Stack trace with row numbers....

System.InvalidOperationException : Nullable object must have a value.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at NHibernate.Engine.BatchFetchQueue.<GetEntityBatch>g__CheckCacheAndProcessResult|25_0(<>c__DisplayClass25_0& ) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\BatchFetchQueue.cs:line 458
   at NHibernate.Engine.BatchFetchQueue.GetEntityBatch(IEntityPersister persister, Object id, Int32 batchSize, Boolean checkCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\BatchFetchQueue.cs:line 442
   at NHibernate.Loader.Entity.BatchingEntityLoader.Load(Object id, Object optionalObject, ISessionImplementor session) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Loader\Entity\BatchingEntityLoader.cs:line 49
   at NHibernate.Persister.Entity.AbstractEntityPersister.Load(Object id, Object optionalObject, LockMode lockMode, ISessionImplementor session) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Persister\Entity\AbstractEntityPersister.cs:line 3736
   at NHibernate.Event.Default.DefaultLoadEventListener.LoadFromDatasource(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:line 349
   at NHibernate.Event.Default.DefaultLoadEventListener.DoLoad(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:line 330
   at NHibernate.Event.Default.DefaultLoadEventListener.Load(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:line 111
   at NHibernate.Event.Default.DefaultLoadEventListener.ProxyOrLoad(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:line 167
   at NHibernate.Event.Default.DefaultLoadEventListener.OnLoad(LoadEvent event, LoadType loadType) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:line 83
   at NHibernate.Impl.SessionImpl.FireLoad(LoadEvent event, LoadType loadType) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 2142
   at NHibernate.Impl.SessionImpl.InternalLoad(String entityName, Object id, Boolean eager, Boolean isNullable) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 1273
   at NHibernate.Type.EntityType.ResolveIdentifier(Object id, ISessionImplementor session) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Type\EntityType.cs:line 365
   at NHibernate.Type.EntityType.Replace(Object original, Object target, ISessionImplementor session, Object owner, IDictionary copyCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Type\EntityType.cs:line 257
   at NHibernate.Type.CollectionType.ReplaceElements(Object original, Object target, Object owner, IDictionary copyCache, ISessionImplementor session) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Type\CollectionType.cs:line 375
   at NHibernate.Type.CollectionType.Replace(Object original, Object target, ISessionImplementor session, Object owner, IDictionary copyCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Type\CollectionType.cs:line 351
   at NHibernate.Type.TypeHelper.Replace(Object[] original, Object[] target, IType[] types, ISessionImplementor session, Object owner, IDictionary copiedAlready) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Type\TypeHelper.cs:line 150
   at NHibernate.Event.Default.DefaultMergeEventListener.CopyValues(IEntityPersister persister, Object entity, Object target, ISessionImplementor source, IDictionary copyCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 468
   at NHibernate.Event.Default.DefaultMergeEventListener.EntityIsDetached(MergeEvent event, IDictionary copyCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 383
   at NHibernate.Event.Default.DefaultMergeEventListener.OnMerge(MergeEvent event, IDictionary copiedAlready) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 181
   at NHibernate.Impl.SessionImpl.FireMerge(IDictionary copiedAlready, MergeEvent event) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 2179
   at NHibernate.Impl.SessionImpl.Merge(String entityName, Object obj, IDictionary copiedAlready) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 821
   at NHibernate.Engine.CascadingAction.MergeCascadingAction.Cascade(IEventSource session, Object child, String entityName, Object anything, Boolean isCascadeDeleteEnabled) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\CascadingAction.cs:line 272
   at NHibernate.Engine.Cascade.CascadeToOne(Object parent, Object child, IType type, CascadeStyle style, Object anything, Boolean isCascadeDeleteEnabled) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 275
   at NHibernate.Engine.Cascade.CascadeCollectionElements(Object parent, Object child, CollectionType collectionType, CascadeStyle style, IType elemType, Object anything, Boolean isCascadeDeleteEnabled) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 294
   at NHibernate.Engine.Cascade.CascadeCollection(Object parent, Object child, CascadeStyle style, Object anything, CollectionType type) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 262
   at NHibernate.Engine.Cascade.CascadeAssociation(Object parent, Object child, IType type, CascadeStyle style, Object anything, Boolean isCascadeDeleteEnabled) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 246
   at NHibernate.Engine.Cascade.CascadeOn(IEntityPersister persister, Object parent, Object anything) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 129
   at NHibernate.Event.Default.DefaultMergeEventListener.CascadeOnMerge(IEventSource source, IEntityPersister persister, Object entity, IDictionary copyCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 513
   at NHibernate.Event.Default.DefaultMergeEventListener.EntityIsDetached(MergeEvent event, IDictionary copyCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 380
   at NHibernate.Event.Default.DefaultMergeEventListener.OnMerge(MergeEvent event, IDictionary copiedAlready) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 181
   at NHibernate.Impl.SessionImpl.FireMerge(IDictionary copiedAlready, MergeEvent event) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 2179
   at NHibernate.Impl.SessionImpl.Merge(String entityName, Object obj, IDictionary copiedAlready) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 821
   at NHibernate.Engine.CascadingAction.MergeCascadingAction.Cascade(IEventSource session, Object child, String entityName, Object anything, Boolean isCascadeDeleteEnabled) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\CascadingAction.cs:line 272
   at NHibernate.Engine.Cascade.CascadeToOne(Object parent, Object child, IType type, CascadeStyle style, Object anything, Boolean isCascadeDeleteEnabled) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 275
   at NHibernate.Engine.Cascade.CascadeCollectionElements(Object parent, Object child, CollectionType collectionType, CascadeStyle style, IType elemType, Object anything, Boolean isCascadeDeleteEnabled) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 294
   at NHibernate.Engine.Cascade.CascadeCollection(Object parent, Object child, CascadeStyle style, Object anything, CollectionType type) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 262
   at NHibernate.Engine.Cascade.CascadeAssociation(Object parent, Object child, IType type, CascadeStyle style, Object anything, Boolean isCascadeDeleteEnabled) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 246
   at NHibernate.Engine.Cascade.CascadeOn(IEntityPersister persister, Object parent, Object anything) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Engine\Cascade.cs:line 129
   at NHibernate.Event.Default.DefaultMergeEventListener.CascadeOnMerge(IEventSource source, IEntityPersister persister, Object entity, IDictionary copyCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 513
   at NHibernate.Event.Default.DefaultMergeEventListener.EntityIsDetached(MergeEvent event, IDictionary copyCache) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 380
   at NHibernate.Event.Default.DefaultMergeEventListener.OnMerge(MergeEvent event, IDictionary copiedAlready) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 181
   at NHibernate.Event.Default.DefaultMergeEventListener.OnMerge(MergeEvent event) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Event\Default\DefaultMergeEventListener.cs:line 55
   at NHibernate.Impl.SessionImpl.FireMerge(MergeEvent event) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 2166
   at NHibernate.Impl.SessionImpl.Merge(String entityName, Object obj) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 895
   at NHibernate.Impl.SessionImpl.Merge[T](T entity) in D:\BuildAgent\work\30546188361a242\src\NHibernate\Impl\SessionImpl.cs:line 901

[fredericDelaporte]

So it seems that idIndex can be null here:

nhibernate-core/src/NHibernate/Engine/BatchFetchQueue.cs

Line 458 in fdac0d0

var indexes = GetSortedKeyIndexes(entityKeys, idIndex.Value, fromIndex, toIndex);

This code is fairly complex, but as I read it, this may happen if there is less than batchsize entities with pending load and the entity to be loaded was not awaiting its load. It looks like this last condition "the entity to be loaded was not awaiting its load" is not expected by the code.

If it is an overlooked case, this code here needs to evolve to support that case.

Otherwise, if that is not supposed to ever happen, there is a bug somewhere else, occurring before this code gets executed.

@maca88, do you remember if it is possible for the batch loader to have to load an entity which was not registered for batch loading?

[RogerKratz]

@fredericDelaporte
Ah, batchsize - good guess! In our failing tests, there was an entity involved with "batch-size=x" set on its <class>, when temporarily remove that, our tests went green.

I wont be in front of a computer more today but if you want to, I can (hopefully) create a failing PR tomorrow.

This was a nasty one. Thanks for your help so far.

[fredericDelaporte]

Yes, a failing PR is always a great help, thanks.

[maca88]

do you remember if it is possible for the batch loader to have to load an entity which was not registered for batch loading?

In all tests that I did, this never happened.

do you have a guess when this could happen?

After some debugging I am able to reproduce the problem with live debugging, here are the steps:

1. Add breakpoints here and here
2. Debug the test BatchableCacheFixture.MultipleGetReadOnlyTest
3. When the code stops at the first breakpoint go to the stacktrace window and double click on the row with name (should be fifth row from top to bottom):

 	NHibernate.dll!NHibernate.Event.Default.DefaultLoadEventListener.ReturnNarrowedProxy(NHibernate.Event.LoadEvent event, NHibernate.Persister.Entity.IEntityPersister persister, NHibernate.Engine.EntityKey keyToLoad, NHibernate.Event.LoadType options, NHibernate.Engine.IPersistenceContext persistenceContext, object proxy) Line 189	C#

4. Wait for VS to load the variables in that region, then click back on the first row in the stacktrace window and hit F5 (continue)
5. Now the code should stop at the second breakpoint and the idIndex will not have the value set

There must be something (like a custom listener) that triggers the removal of the loadable entity keys which happens between AddBatchLoadableEntityKey and GetEntityBatch calls.
The solution is probably to modify the GetEntityBatch and GetCollectionBatch to support this scenario.

EDIT:

Thankfully I have still the fiddle that compares the old and new algorithm and with the old one it works without any exception (by using the inputs with I was able to reproduce the error), but I am still sceptic if the returned array is correct as it contains the id that was not on the entity loading list. I will dig deeply into it to find what exactly can cause this and try to provide a solution.

[RogerKratz]

@maca88 As stated above, this happened to me when having "batch-size" set on the class mapping, if that helps you.

Hopefully I can write a failing test tomorrow (not in front of a computer atm).

[RogerKratz]

@maca88 @fredericDelaporte
Failing test here, #1924.

I tried to apply a fix as well but current impl is quite (too?) complex so I gave it up quickly and left that part to you who knows this code base better than me... Please tell me if you want me to continue to find a fix for this issue.

[fredericDelaporte]

I agree this implementation is a bit too complex in my opinion. Note it has not been introduced by 09120c0, it was already there previously. 09120c0 has refactored it though.