Unable to serialize session after modifying the index of entities in a list

Description

1. Map an entity that contains a collection of child entities as list.
2. Reorder some child entries in the list or remove one, so that the later ones need to get a new index.
3. Flush the session and commit the transaction, but don't close the session.
4. Try to serialize the session.

This results in the following exception:
System.InvalidOperationException : Cannot serialize a Session while connected

The message of the exception is wrong. The session isn't connected. The real problem is that the AbstractBatcher still has entries in the _commandsToClose list and therefore returns true in the HasOpenResources property, which causes the ConnectionManager to return false in the IsReadyForSerialization property.

This is a regression in NHibernate 4.0 and is caused by NH-3072. To be more specific, the removal of the "if (st == null)" condition caused it. Since this was a pull request from me, I'd like to not just revert it without understanding what happens first.

Environment

None

Activity

Show:
cremor
October 30, 2014, 9:22 AM

Yeah. As written, I even know how to fix it, I just have to re-add that if condition. But that code doesn't make any sense for me and I think just re-adding the if condition is a wrong solution.

The method OneToManyPersister.DoUpdateRows could create many statements (DbCommands) since there are two loops (one for deletes, one for inserts) that call session.Batcher.PrepareCommand for each entry to be updated. But the finally block only closes one command (the last one). Isn't this the real bug? Should we save all created commands so we can then close them all?

The only reason why this doesn't happen for the insert loop too (and therefore already happend before my changes in NH-3072) is that there useBatch seems to be always true, which leads to a call of session.Batcher.PrepareBatchCommand() which reuses the existing command since both the SQL and parameter types are equal.

cremor
October 30, 2014, 10:36 AM

Maybe even better than holding a list of created commands would be to move the try-catch-finally blocks inside the loops. Then each command would be closed right after it is executed. That's also what's done in BasicCollectionPersister.DoUpdateRows.

cremor
October 30, 2014, 11:56 AM

I've now pushed an additional test (for maps) and a fix to the pull request.

Alex Zaytsev
January 20, 2015, 8:20 AM

Merged to 4.0.x at 569b49c4ead3f026f0a161c1b9b6833fb92e7216

Alex Zaytsev
May 1, 2017, 3:37 AM

Closing issues resolved in 4.0.3

Assignee

Alex Zaytsev

Reporter

cremor

Components

Fix versions

Affects versions

Priority

Critical
Configure