[alias-server] Prevent reentrency in handleBlockConnected
ClosedPublic
Actions

Authored by bytesofman on Apr 12 2023, 17:00.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC21a1e99e684d: [alias-server] Prevent reentrency in handleBlockConnected

Summary

T3060

Implement async-lock to prevent reentrancy in handleBlockConnected

If handleBlockConnected is called by parseWebsocketMessage and is already processing, the next call will wait until the first call completes

Test Plan

Review new unit test
npm test
Note: this unit test is something of a stub right now. It will matter more when we are actually updating and changing the database. This is where we will be able to actually verify no reentrancy.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

prevent-re-entrancy

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 23247
Build 46113: Build Diff	alias-server-tests
Build 46112: arc lint + arc unit

Event Timeline

bytesofman created this revision.Apr 12 2023, 17:00

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 12 2023, 17:00

bytesofman requested review of this revision.Apr 12 2023, 17:00

Harbormaster completed remote builds in B23247: Diff 39639.Apr 12 2023, 17:32

Fabien requested changes to this revision.Apr 12 2023, 19:35

Fabien added a subscriber: Fabien.

Fabien added inline comments.

apps/alias-server/src/chronikWsHandler.js
66	I don't feel like this log is very useful. I tend the keep the logs minimalist and mostly returning the errors
apps/alias-server/test/chronikWsHandlerTests.js
191	If you want to test the lock you probably need to return false so it loops for some time

This revision now requires changes to proceed.Apr 12 2023, 19:35

remove debug logging

apps/alias-server/src/chronikWsHandler.js
66	Yes, this is a debug log so I could check if the dependency was even working. More important to check this when we have actual async stuff happening in the loop where reentrancy would have an impact. Removed.
apps/alias-server/test/chronikWsHandlerTests.js
191	For now, this still doesn't really matter. Both loops will still return the correct info. Later on this test will be expanded to include database reads and writes, so that reentrancy would return a different value if it occured.