Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add required locking (mutex) to GetMempoolTx #403

Merged
merged 1 commit into from
Jul 21, 2022

Conversation

LarryRuane
Copy link
Collaborator

@LarryRuane LarryRuane commented Jul 15, 2022

Closes #402

To reproduce the problem, call the GetMempoolTx gRPC concurrently. For example, run the following in two shell windows simultaneously:

while :;do grpcurl -plaintext localhost:9067 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetMempoolTx;done >/dev/null

The lightwalletd instance should panic within several seconds.

Exception has occurred: panic
"runtime error: invalid memory address or nil pointer dereference"
Stack:
	 4  0x0000000000dc387a in github.com/zcash/lightwalletd/frontend.(*lwdStreamer).GetMempoolTx
	     at /g/lightwalletd/frontend/service.go:479
	 5  0x0000000000d9248a in github.com/zcash/lightwalletd/walletrpc._CompactTxStreamer_GetMempoolTx_Handler
	     at /g/lightwalletd/walletrpc/service_grpc.pb.go:632

The problem is that mempoolMap may have been changed (by another thread) during the current thread's single execution of this function, so that when it's indexed by txid at line service.go:479, that key may not exist.

I looked for other (unreported) instances of this problem and didn't find any, although did find that GetLatestBlock() has a "logical" race condition; it may return a height and hash that don't correspond to each other since they're fetched separately without locking. So this PR also fixes this problem.

Copy link
Contributor

@pacu pacu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

@LarryRuane LarryRuane force-pushed the 2022-07-GetMempoolTx-locking branch from 286e3b7 to 7731ae8 Compare July 21, 2022 17:29
@LarryRuane
Copy link
Collaborator Author

Force-pushed rebased

@LarryRuane LarryRuane merged commit 2d3943b into zcash:master Jul 21, 2022
@LarryRuane LarryRuane deleted the 2022-07-GetMempoolTx-locking branch July 21, 2022 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mempool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mempool tx.Hash nil pointer dereference in frontend/service.go:480
2 participants