bitshares / bitshares/bitshares-core

Race condition in P2P listen_to_p2p_network()

Closed
#2,834 0 comments 0 reactions 0 assignees Claimed by @abitmore View on GitHub
Dominant language
C++
Stars
1.2k
Forks
660
Avg merge
8h 17m
Merged PRs (30d)
26

Description

**Bug Description**
During testing there is a race condition in `listen()` in `node_impl::listen_to_p2p_network()` that appears quite frequently (see Github Actions logs e.g. https://github.com/bitshares/bitshares-core/actions/runs/34021597393/job/101455080081?pr=2833). The "bind - close (via object destruction) - rebind" approach is not ideal. A better approach is to not close the server socket if the first bind is successful, but keep and use it directly.

Ideally we also need a unit test case to cover the fix:
- if bind succeeds
- if bind fails
- if "p2p-endpoint" option is not set, retry port 0 one time
- if "p2p-endpoint" option is set, wait for a few seconds and then retry, repeat

Commit https://github.com/bitshares/bitshares-core/commit/8db58de74e6dc43552ec5165bf53d2879cf80b44 in https://github.com/bitshares/bitshares-core/pull/1764 is related.

**Impacts**
Describe which portion(s) of BitShares Core may be impacted by this bug. Please tick at least one box.
- [ ] API (the application programming interface)
- [ ] Build (the build process or something prior to compiled code)
- [ ] CLI (the command line wallet)
- [ ] Deployment (the deployment process after building such as Docker, Travis, etc.)
- [ ] DEX (the Decentralized EXchange, market engine, etc.)
- [x] P2P (the peer-to-peer network for transaction/block propagation)
- [ ] Performance (system or user efficiency, etc.)
- [ ] Protocol (the blockchain logic, consensus, validation, etc.)
- [ ] Security (the security of system or user data, etc.)
- [ ] UX (the User Experience)
- [ ] Other (please add below)

**Steps To Reproduce**
Github Actions

**Expected Behavior**

**Screenshots (optional)**

**Host Environment**
- Host OS: Ubuntu 24.04 LTS
- Host Physical RAM
- BitShares Version: 7.0.2
- OpenSSL Version: 3.0
- Boost Version: 1.83

**Additional Context (optional)**

## CORE TEAM TASK LIST
- [ ] Evaluate / Prioritize Bug Report
- [ ] Refine User Stories / Requirements
- [ ] Define Test Cases
- [ ] Design / Develop Solution
- [ ] Perform QA/Testing
- [ ] Update Documentation

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at node_impl::listen_to_p2p_network() and its listen() path, then review the related commit 8db58de74e6dc43552ec5165bf53d2879cf80b44 and the linked GitHub Actions failure. Trace bind and socket ownership behavior, and define tests for successful bind, failed bind with and without p2p-endpoint, and retry behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.