Fix transaction finalizer causing LiteDB.LiteException/System.ObjectDisposedException

[ejdre-vestas]

I believe that when #1906 was introduced, a bug was also added.

When the transaction is finalized, the code is calling _monitor.ReleaseTransaction, and will eventually do: https://github.com/litedb-org/LiteDB/blob/48d89fd515f3ee348b13c5248d636bedd3a71611/LiteDB/Engine/Services/TransactionMonitor.cs#L116-L119

The finalizers always run on a specific thread controlled by the Garbage Collector, so the thread that is releasing the transaction will never have acquired a lock.

I was running version 5.0.15, which is before this band-aid fix #2280 (two of the referenced issues also contain stack traces starting on a Finalize), so I was able to consistently get an exception anytime the Finalize was called.

~~Even though the exception is hidden on newer versions, it still makes sense to me to have this fixed.~~ Edit: The new version will still throw an exception in another place, so this PR is still required to fix problems calling finalizers. 2 exceptions still come up without this fix:

Unhandled exception. System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'System.Threading.ThreadLocal`1[[LiteDB.Engine.TransactionService, LiteDB, Version=6.0.0.0, Culture=neutral, PublicKeyToken=null]]'.
   at System.Threading.ThreadLocal`1.GetValueSlow()
   at LiteDB.Engine.TransactionMonitor.ReleaseTransaction(TransactionService transaction) in /home/edureis95/repo/LiteDB/LiteDB/Engine/Services/TransactionMonitor.cs:line 145
   at LiteDB.Engine.TransactionService.Dispose(Boolean dispose) in /home/edureis95/repo/LiteDB/LiteDB/Engine/Services/TransactionService.cs:line 446
   at LiteDB.Engine.TransactionService.Finalize() in /home/edureis95/repo/LiteDB/LiteDB/Engine/Services/TransactionService.cs:line 79
   at System.GC.RunFinalizers()

Unhandled exception. LiteDB.LiteException: current thread must contains transaction parameter
   at LiteDB.Constants.ENSURE(Boolean conditional, String message) in /home/edureis95/repo/LiteDB/LiteDB/Utils/Constants.cs:line 146
   at LiteDB.Engine.TransactionMonitor.ReleaseTransaction(TransactionService transaction) in /home/edureis95/repo/LiteDB/LiteDB/Engine/Services/TransactionMonitor.cs:line 145
   at LiteDB.Engine.TransactionService.Dispose(Boolean dispose) in /home/edureis95/repo/LiteDB/LiteDB/Engine/Services/TransactionService.cs:line 446
   at LiteDB.Engine.TransactionService.Finalize() in /home/edureis95/repo/LiteDB/LiteDB/Engine/Services/TransactionService.cs:line 79
   at System.GC.RunFinalizers()

[JKamsker]

Thank you for that! Please target your pr at dev and provide atleast one regression test if possible.

[JKamsker]

@codex review

[chatgpt-codex-connector[bot]]

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ejdre-vestas]

Thank you for that! Please target your pr at dev and provide atleast one regression test if possible.

I have added the test, and if you switch the code back to call _monitor.ReleaseTransaction(this);, the test will fail with UnhandledException mentioned in the PR description. I know it's annoying that this is unhandled, but the GC thread "swallows" the exception, so you cannot catch it.

[ejdre-vestas]

Even though I made this fix, I think the overall best change is to remove the finalizer completely.

Please someone correct me if I'm wrong but the conclusion that led to this implementation (written in https://github.com/litedb-org/LiteDB/issues/1772#issuecomment-751338720), is not valid.

The author wanted to remove a TransactionService from the TransactionMonitor when a thread ends. From his sentence, he believed that the thread ending would trigger the finalizer. However, that's not how GC does things. The finalizer will only ever be called when there are no longer any strong references to an object, and TransactionMonitor holds a strong reference to all TransactionServices. So, essentially, the finalizer will never be called unless the TransactionService is removed from TransactionMonitor (or both are ready to be collected by the GC). If the whole point of the finalizer is to perform this removal, you can see where I'm getting at; it will never do what it was supposed to.

@JKamsker please let me know how I should proceed.