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

coot/network memory leak #762

Closed
wants to merge 2 commits into from
Closed

coot/network memory leak #762

wants to merge 2 commits into from

Conversation

coot
Copy link
Contributor

@coot coot commented Apr 4, 2020

Issue

This PR updates for changes in coot/network-memory-leak branch in
ouroboros-network, and fixes a memory leak in cardano-node.

  • This PR results/does not result in breaking changes to upstream dependencies.

Checklist

  • This PR contains all the work required to resolve the linked issue.

  • The work contained has sufficient documentation to describe what it does and how to do it.

  • The work has sufficient tests and/or testing.

  • I have committed clear and descriptive commits. Be considerate as somebody else will have to read these.

  • I have added the appropriate labels to this PR.

coot added 2 commits April 4, 2020 12:55
The TVar should be StrictTVar, this is left as a todo.
@coot coot added bug Something isn't working byron Required for a Byron mainnet: replace the old core nodes with cardano-node. networking Issues and PRs related to networking labels Apr 4, 2020
@coot coot marked this pull request as ready for review April 7, 2020 18:43
@coot coot requested review from dcoutts and Jimbo4350 and removed request for dcoutts April 7, 2020 18:44
@coot
Copy link
Contributor Author

coot commented Apr 7, 2020

We need first merge IntersectMBO/ouroboros-network#1902

Comment on lines +223 to +224
-- TODO: use StrictTVar
atomically $ writeTVar varTip $! tip
Copy link
Contributor

Choose a reason for hiding this comment

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

This patch is no longer necessary. The node top level no longer reads the tip at all for use in tracers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used it today to trace epoch boundary into eventlog, but this could be done in consensus as well (it was just simpler to do that here, in bare IO :) with out any abstract constraints)

@coot coot closed this Apr 8, 2020
@erikd erikd deleted the coot/network-memory-leak branch August 18, 2022 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working byron Required for a Byron mainnet: replace the old core nodes with cardano-node. networking Issues and PRs related to networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants