Skip to content

fix(tokens,extended-typeorm): release locks when an update or transaction fails - #13

Open
Kshot3000 wants to merge 1 commit into
rosen-bridge:devfrom
Kshot3000:fix/release-locks-on-failure
Open

Kshot3000 wants to merge 1 commit into
rosen-bridge:devfrom
Kshot3000:fix/release-locks-on-failure

Conversation

@Kshot3000

Copy link
Copy Markdown

Summary

Two items from rosen-bridge/guard-service#28 ("a lock that is not released on every path") live in this repo. Both turn a single transient failure into a permanent hang:

  • TokenMap.updateConfigByJson (@rosen-bridge/tokens) released its semaphore only on success. After one corrupted config (CorruptedConfigError) or one throwing callback, every later updateConfigByJson on the same TokenMap waits forever.
  • CustomQueryRunner (@rosen-bridge/extended-typeorm) kept its mutex when rollbackTransaction rejected — and also when startTransaction failed after acquiring it. Since the driver hands out a single shared runner, one failure blocked every later transaction on that data source.

Fix

  • TokenMap: run the update through the semaphore's own use() helper, which releases on both success and error. No behavior change otherwise — config is still only assigned after full validation, and callbacks still run inside the lock.
  • CustomQueryRunner.startTransaction: release the just-acquired mutex if the transaction never started (nothing else can release it), and only store the releaser on success.
  • CustomQueryRunner.rollbackTransaction: release in a finally — the transaction is over either way, so keeping the mutex only bricks the runner.
  • CustomQueryRunner.commitTransaction is deliberately unchanged: when a commit fails, the transaction is still active and the caller's rollback (this is exactly what TypeORM's EntityManager.transaction does) is what ends it and releases the mutex. Releasing in commit would make that rollback throw Cannot rollback transaction before starting it and leave the transaction open. I also initialized the releaseMutex field to null to match its declared type — it was undefined until the first transaction.

Tests

Tests first — each new test fails on the current code and passes with the fix:

  • tokens: after a corrupted-config update throws, a valid update still completes; same after a registered callback throws (both raced against a timeout — on current code they never resolve). Package suite 38/38.
  • extended-typeorm: a failed start (unsupported SQLite isolation level, which throws after the mutex is acquired) and a failed rollback (transaction ended by a raw COMMIT behind the runner's back) both leave releaseMutex === null and the mutex unlocked, and a valid transaction still starts and commits afterwards. Package suite 4/4.

Dependents re-checked: root npm run build clean, extended-tokens 12/12, rosen-extractor 228/228, tx-pot 50/50. tsc --noEmit clean for both packages; eslint/prettier clean. Changeset included (patch for both packages).

The remaining items from guard-service#28 live in scanner (abstractErgoExtractor.unhook) and sign-protocols (TssSigner) — not covered here.

…tion fails

TokenMap.updateConfigByJson released its semaphore only on success, so a
single corrupted config or throwing callback blocked every later update
forever. It now runs through Semaphore.use, which releases on every path.

CustomQueryRunner kept its mutex when startTransaction or
rollbackTransaction failed, blocking every later transaction on the
runner. Both now release on failure. Commit still releases only on
success on purpose: a failed commit leaves the transaction active, and
the caller's rollback is what ends it and releases the mutex.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant