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

Fix deadlock issue with stdout tracer in tx-generator #5460

Merged
merged 4 commits into from
Sep 6, 2023

Conversation

jutaro
Copy link
Contributor

@jutaro jutaro commented Sep 6, 2023

Problem:
The root issue we encountered with the tx-generator was the creation of a new stdout backend for every tracer instance. This approach, however, conflicts with the fundamental requirement of having only one stdout backend per application. Consequently, this practice led to unnecessary garbage collection of these backends and resulted in the dreaded "BlockedIndefinitelyOnMVar" exception.

Solution:
To rectify this issue, the proposed solution involves constructing and utilizing only a single backend for all tracers. The introduction of a shutdown handler, in this context, becomes redundant. It's important to note that a shutdown handler wouldn't actually address the underlying problem but would merely mask it.

Alternative Consideration:
While it is conceivable to enforce a single instance from the trace-dispatcher library, this approach would necessitate the use of unsafePerformIO. We've chosen not to adopt this method due to concerns related to its contravariant interface.

This pull request aims to implement the solution by ensuring a single backend, thus resolving the issue and enhancing the stability of the tx-generator.

In addition we adopted the tracing of tx-generator to use the new interface, where it was possible. So the fix itself can be found in function initTxGenTracers.

@jutaro jutaro self-assigned this Sep 6, 2023
@jutaro jutaro marked this pull request as ready for review September 6, 2023 09:41
@jutaro jutaro requested review from a team as code owners September 6, 2023 09:41
* avoid double call to initTxGenTracers
* use new tracing interface if possible
@jutaro jutaro force-pushed the jutaro/deadlock-stdout-thread-tx-generator branch from eaacd22 to 5f84d5f Compare September 6, 2023 12:20
@NadiaYvette NadiaYvette force-pushed the jutaro/deadlock-stdout-thread-tx-generator branch 2 times, most recently from 5f62320 to b8094d9 Compare September 6, 2023 16:12
A version message is already emitted by initTxGenTracers. This is
also a cleanup, as breaking the Action abstraction is ugly. Minor
note: traceTxGeneratorVersion is no longer used.
@NadiaYvette NadiaYvette force-pushed the jutaro/deadlock-stdout-thread-tx-generator branch from b8094d9 to f4cbcb8 Compare September 6, 2023 16:14
Copy link
Contributor

@mgmeier mgmeier left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@mgmeier mgmeier added this pull request to the merge queue Sep 6, 2023
Merged via the queue into master with commit 4139153 Sep 6, 2023
23 of 24 checks passed
@mgmeier mgmeier deleted the jutaro/deadlock-stdout-thread-tx-generator branch September 6, 2023 20:49
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.

None yet

3 participants