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

bugfix - remove deadlock when pinging bootnodes #142

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

lithp
Copy link
Contributor

@lithp lithp commented Oct 15, 2021

Fixes a deadlock which occurs during boot.


Before:

Oct 15 13:58:05.390  INFO trin_core::jsonrpc::service: listening for commands: /tmp/trin-jsonrpc2.ipc    
Oct 15 13:58:05.491  WARN trin_core::portalnet::events: [trin-core/src/portalnet/events.rs:31] took more than 100 ms to acquire lock    
Oct 15 13:58:05.491  WARN trin_core: [trin-state/src/network.rs:38] lock held for over 100 ms    
Oct 15 13:58:05.491  WARN trin_core::portalnet::overlay: [trin-core/src/portalnet/overlay.rs:272] took more than 100 ms to acquire lock    
[blocks forever]

After:

Oct 15 15:12:43.300 DEBUG trin_state::network: Attempting bond with bootnode ENR: NodeId: 0x76f9..6f28, Socket: Some(71.202.127.37:4567)    
Oct 15 15:12:43.300 DEBUG discv5::service: Sending RPC Request: id: 41c978172b0c39d3: TALK: protocol: 7374617465, request: 0101000000000000000c00000001000000ffffffffffffffff0000000000000000000000000000000000000000000000002400000000000000 to node: Node: 0x76f9..6f28, addr: Some(71.202.127.37:4567)
Oct 15 15:12:43.402  WARN trin_core: [trin-core/src/portalnet/overlay.rs:279] lock held for over 100ms, not yet released    
Oct 15 15:12:44.303 DEBUG discv5::service: RPC Request timed out. id: 41c978172b0c39d3
Oct 15 15:12:44.304  WARN trin_core: [trin-core/src/portalnet/overlay.rs:279] lock held for too long: 1003ms    
Oct 15 15:12:44.304 DEBUG trin_state::network: Timed out while bonding with ENR: NodeId: 0x76f9..6f28, Socket: Some(71.202.127.37:4567)    

Comment on lines 38 to 43
let table_entries = rw_read!(self.overlay.discovery).discv5.table_entries_enr();
for enr in table_entries {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So I take it that the old lock was being held for the whole iteration, and now it's not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's my impression as well. But I don't have any special knowledge as to how rust works, I made a guess that this would cause Rust to drop the lock sooner and it seems to have worked!

@lithp lithp changed the base branch from lithp/add-lock-debug-logging to master October 22, 2021 16:51
@lithp lithp force-pushed the lithp/fix-deadlock branch from f32c07f to bda47df Compare October 22, 2021 17:14
@lithp
Copy link
Contributor Author

lithp commented Oct 22, 2021

Okay, now that the other PR has been merged this one is ready for review!

Copy link
Collaborator

@carver carver left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@njgheorghita njgheorghita left a comment

Choose a reason for hiding this comment

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

🚢

@lithp lithp merged commit c6b77ab into ethereum:master Oct 22, 2021
@lithp lithp deleted the lithp/fix-deadlock branch October 22, 2021 17:30
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.

3 participants