Skip to content

Listen to p2p network without using a temp server - #2861

Open
abitmore wants to merge 12 commits into
developfrom
p2p-listen-fix
Open

Listen to p2p network without using a temp server#2861
abitmore wants to merge 12 commits into
developfrom
p2p-listen-fix

Conversation

@abitmore

@abitmore abitmore commented Sep 10, 2026

Copy link
Copy Markdown
Member

PR for #2834 (unstably reproduces with test cases in #2862).

Main changes:

  • Updated set_reuse_address() functions in FC, do not set SO_REUSEPORT (see Update set_reuse_address bitshares-fc#260).
  • Updated node_impl::listen_to_p2p_network(), no longer use the "bind without SO_REUSEADDR to check port availability/close/set SO_REUSEADDR/bind for real" mode, instead use the "set SO_REUSEADDR/bind and retain" mode.
  • Updated tests to avoid the "bind to port 0/get port/close/rebind to the port" mode, but use the "bind to port 0/get port/retain the socket" mode.
  • Added tests.

abitmore and others added 2 commits September 11, 2026 17:37
This partially reverts commit 8db58de
("Update tests to get around race conditions in code"): the changes in
tests were reversed but the comments in code remain unchanged.
A failing test in CI said this, and only this:

    unknown location(0): fatal error: in "authority_tests/proposal_delete": unknown type

"unknown type" is not a diagnosis. It is the literal string Boost.Test prints from its
catch-all in execution_monitor.ipp when an exception reaches it that is neither a
std::exception nor any type it knows. fc::exception derives from nothing at all, so every
fc exception that escapes a test case lands there and is reported identically -- no code,
no message, no location.

Worse, the exception's own text is never written anywhere, so it cannot be recovered from
the log afterwards. Checked: the fourteen lines preceding the failure are plugin-shutdown
logging and nothing else. No amount of extra log capture in the workflow would have
produced the message, because the message never existed.

That made the report actively misleading rather than merely thin. Running the same test
binaries locally produced the identical line from a completely unrelated cause -- no
Elasticsearch listening on 9200 -- which is how it became clear the string carries no
information about what actually went wrong.

Registering a translator turns it into fc's detail string. Verified against the
Elasticsearch case, where the cause was already known, so the new output could be checked
for being right rather than merely different:

    es->check_status(): ES database is not up in url http://127.0.0.1:9200/
        {"url":"http://127.0.0.1:9200/"}
        th_a  elasticsearch_plugin.cpp:544 init_program_options
    rethrow
        {}
        th_a  database_fixture.hpp:651 init

The header is included by all five test binaries, so all of them gain it. One cost worth
naming: Boost now reports the failure at the translator's own line rather than
"unknown location(0)". The real site is in the fc chain underneath, which is more than was
there before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@abitmore abitmore linked an issue Sep 11, 2026 that may be closed by this pull request
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In development

Development

Successfully merging this pull request may close these issues.

Race condition in P2P listen_to_p2p_network()

2 participants