LightningEnvironment finalizer exception

[p-lindberg]

I'm curious about the following exception, thrown inside the Dispose method of LightningEnvironment:

Lightning.NET/src/LightningDB/LightningEnvironment.cs

Lines 483 to 484 in 40e4d5a

	if(!disposing)
	throw new InvalidOperationException("The LightningEnvironment was not disposed and cannot be reliably dealt with from the finalizer");

What is the rationale behind it? Is it only because there is no guarantee that it's safe to dispose the environment from a finalizer?

In my use case, which is inside a Unity game, it causes some issues. We don't always have control over when finalizers are called due to the fact that the runtime can hot-reload assemblies. When this happens, all objects are finalized immediately. This triggers the exception, unless we take steps to detect the assembly reload and manually dispose before it happens (which is my current workaround).

What's more; the exception itself just gets swallowed by the runtime, as, at least in the Mono runtime, exceptions in finalizers are not propagated anywhere. The only way to notice that it actually occurs is to have the debugger connected and place a breakpoint on the exception. Luckily, I noticed this while debugging something else. The unfortunate effect of this is that the remainder of the disposal method does not run, which leaves the environment open. This could potentially be an even less desirable outcome than what might happen if the environment is closed at an inopportune moment.

Would you be open to reconsidering this approach? I believe it may be better to just proceed with the disposal regardless, as that would at least result in a best-effort cleanup in all cases.

[CoreyKaylor]

Yes, there is no guarantee that it's safe to dispose the environment and all other resources within the environment due to the non-deterministic nature of the finalizer. It is necessary to call Dispose to properly release resources and not potentially corrupt the database.

[p-lindberg]

Ah, I see why it makes sense to do this if the database could be corrupted by it. My impression was that it's generally safe to close the environment whenever, but that accessing a transaction or other resource after doing so would lead to a segfault. The official docs states that this "will" happen, but not sure if that's really definite, or if something else may fail before that. I suppose any outstanding transactions would fail to complete, which could leave things in an undefined state.

In that case, I'm not sure what the best solution may be. The current behaviour is kind of unfortunate (in Unity at least), but may be favourable over the alternative. Perhaps it would be possible to track all disposable resources like transactions inside the environment object and forcibly dispose them if necessary when the environment is finalized, but that sounds like a big change, with potential other drawbacks.

[CoreyKaylor]

I'm not all that familiar with Unity. There's no deterministic way to call dispose when using it?

[p-lindberg]

The problem with Unity is that it can reload the entire application domain at any time when running in the context of the editor. As far as I understand it, they tear down the current state of the runtime (finalising all objects) then reload all assemblies and start everything up again. This is not initiated from client code, so there's no natural place to handle disposal of resources beforehand. This is usually not an issue though, since most resources ensure that Dispose is called from the finalizer if it hasn't already been called.

My workaround was to hook in to their AssemblyReloadEvents.beforeAssemblyReload event and call dispose on the environment. It's not super pretty though, since in principle this could happen while the database is being used. In practice that's not very likely though, since assembly reloads only happen while the game is not running.

[p-lindberg]

In essence, the issue is that there's no obvious way to ensure proper cleanup under these circumstances. :/

If I want to be really thorough and make sure that there is no probability of undefined behaviour here, I would probably need to do something like I mentioned above: track all the created transactions / cursors / etc, and ensure that they are "force-disposed" by the same mechanism, before the environment. On the other hand, it's likely more work than it's worth (for me), given that I can be reasonably confident that it won't cause problems in practice, and knowing that it only affects our testing environment (the game editor) if improper cleanup were to happen.

[CoreyKaylor]

FWIW, the transactions do check if the environment is already disposed. So worst case is it would not be a committed transaction which may be what you want anyways, not sure.

[CoreyKaylor]

I would be open to an option of not throwing, but keeping the default to throw if that would help your scenario.

[p-lindberg]

FWIW, the transactions do check if the environment is already disposed. So worst case is it would not be a committed transaction which may be what you want anyways, not sure.

In that case, it sounds like my current solution should be safe enough.

I would be open to an option of not throwing, but keeping the default to throw if that would help your scenario.

I'd be wary to add it if it only serves my use case - especially considering I found a decent workaround. Might be best to just leave it as is.