Fix committing or rollbacking dangling transactions

[CMCDragonkai]

Description

In the EFS, we came across a problem where there is a dangling transaction that is still pending. It may be neither committed, nor rollbacked, or it may be in the process of committing or rollbacking.

Either way, when the DB.stop() is called, it proceeds to rollback all existing transactions in the transaction reference set.

Ideally, all transactions should be committed or rollbacked before the await db.stop() is called. However if there are dangling transactions there should be rollbacked if their status is neither committed or rollbacked, but if they have a particular status, then the DB.stop should be waiting for those pending statuses to finish.

Here we introduce a lock to be used between DBTransaction.destroy, DBTransaction.commit and DBTransaction.rollback. The basic idea is for destroy to wait on a commit or rollback to finish. This allows one to call await transaction.destroy() while the committing or rollbacking is occurring.

However at the same time, if a transaction is pending to commit, but hasn't started committing. It's possible for the DB.stop() to already start rollbacking. If this occurs, during the resource release, the transaction will attempt to commit, in that case, an exception still occurs because the transaction is already rollbacked. This is a legitimate exception.

Issues Fixed

• Related: ci: merge staging to master js-encryptedfs#74 (comment)

Tasks

• 1. Ensure that DB.stop() only performs await transaction.rollback when the transaction is neither committing nor rollbacking. If it is in the middle of comitting or rollbacking then it should just wait for it complete.
• 2. Add a test for dangling transactions for tests/DBTransaction.test.ts
• 3. Remove await this.destroy() from DBTransaction.commit and DBTransaction.rollback, this is done automatically be the resource release

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

• Make big changes easier — review code in small groups of related files

• Know where to start — see the whole change at a glance

• Take a code tour — explore the change with an interactive tour

• Make comments and review — all fully sync’ed with github

  Try it now!

Legend

[CMCDragonkai]

@tegefaulkes the exception in EFS may still occur simply because the EFS could stop the database before the committing or rollbacking has even started.

In that case the dangling transaction attempts to release which will either commit or rollback. But this will cause an exception. This is unavoidable UNLESS we were to make is that committ or rollback is a noop if it is already committing or rollbacking.

[CMCDragonkai]

The DBTransaction.committed and DBTransaction.rollbacked is a bit of a misnomer. These 2 actually mean that it has started committing or rollbacking. So I'm changing these to be committing and rollbacking instead to be more obvious.

[CMCDragonkai]

In that case the dangling transaction attempts to release which will either commit or rollback. But this will cause an exception. This is unavoidable UNLESS we were to make is that committ or rollback is a noop if it is already committing or rollbacking.

So this change only applies to the case where committing or rollbacking has already started, and in which case those transactions should not result in any problems.

I'm not sure if that will fix the EFS, or we should apply a fix in EFS to ensure that the transaction is in fact committed or rollbacked before you stop the EFS (this is the better solution).

[CMCDragonkai]

Ok for now the EFS will still need to ensure that they are in fact committing or rollbacking any dangling transactions. But the db.stop() here is now more robust. @tegefaulkes