Skip to content

bug: flaky test: race condition in crawler_peer_limit_one_connect_ok_then_drop test #11373

Description

@mpguerra

Problem

The test crawler_peer_limit_one_connect_ok_then_drop in zebra-network::peer_set::initialize::tests::vectors is failing with a race condition. The test is flaky because the address book updater task is shut down before all in-flight messages from the crawler are processed.

Error Output

stopping address book updater error=Err(AllAddressBookUpdaterSendersClosed)
test peer_set::initialize::tests::vectors::crawler_peer_limit_one_connect_ok_then_drop ... FAILED

Root Cause

The crawl_and_dial function (line 958 in zebra-network/src/peer_set/initialize.rs) spawns background tasks that send updates to the address book updater. When the test terminates, these spawned tasks may still have pending messages in flight. If the address book updater channel is closed before these messages are processed, the crawler fails with AllAddressBookUpdaterSendersClosed, causing the test to fail.

This is a classic race condition: the test's assertions/cleanup happen before the spawned background tasks have finished their work.

Solution

Add synchronization to allow in-flight background tasks to complete before the test terminates. This should be done in the test at the point where the connection lifecycle is complete but before assertions and cleanup occur.

Option 1 (Recommended): Add a short yield + sleep to allow pending tasks to drain:

// Allow pending background tasks to process messages
tokio::task::yield_now().await;
tokio::time::sleep(Duration::from_millis(100)).await;

Option 2: Mock or disable the address book updater if it's not essential to the test's purpose. If the test is specifically about peer connection limits, the address book updater may not need to be fully functional.

Option 3: Refactor the test to use a custom address book updater mock that properly handles channel closure during shutdown.

Files Affected

  • zebra-network/src/peer_set/initialize.rs - test module, specifically the crawler_peer_limit_one_connect_ok_then_drop test in the tests::vectors section

Context

This is related to Zebra's async task coordination during peer connection management. The peer crawler uses spawned tokio tasks to handle connection attempts and address book updates concurrently. Proper cleanup order is critical to avoid dropping channels while tasks are still trying to send on them.

Related Architecture

Per Zebra's Tower Service pattern and async concurrency guidelines:

  • All external waits need timeouts and must be cancellation-safe
  • Progress tracking should use "time since last change" to detect stalls
  • Task spawning requires careful cleanup synchronization to prevent race conditions

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions