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.

Sign up for GitHub

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

Jump to bottom

eth: fix race condition in eth/sync tests #522

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

eth: fix race condition in eth/sync tests #522

wants to merge 3 commits into from

Commits on Feb 7, 2023

  1. eth: fix race condition in tests 

    The race was discoverable with
go test -race ./eth
or under 'slow' machine conditions, eg. GOMAXPROCS=1
and/or a 'slower' machine.

The race was around the minArtificialFinalityPeers
value, which was both
- assigned in (during) the tests, and
- checked in the nextSyncOp function

and the order of those uses was nondeterministic.

This patch resolves the issue by moving
the adhoc and temporary re-assignment of that
package-wide value to the start of the tests,
before any go routines for peer handling
or sync protocol are fired off.

Note that the real-clock timeouts in these
tests can still cause spurious test failures
under 'slow' conditions (see their 250ms sleeps).

Fixes #521

Date: 2023-01-30 13:39:38-08:00
Signed-off-by: meows <[email protected]>
    @meowsbits
    meowsbits committed Feb 7, 2023
    Configuration menu
    Copy the full SHA
    51da9a4 View commit details
    Browse the repository at this point in the history

  2. eth: move chainSync.forced state type to atomic instead of bool 

    @ziogaschr found a(nother) race condition
in the tests, which I have had a hard time dupliating
with
env GOMAXPROCS=1 go test -count 1 -race ./eth

But this should, hopefully, fix the issue.

Date: 2023-02-07 08:47:44-08:00
Signed-off-by: meows <[email protected]>
    @meowsbits
    meowsbits committed Feb 7, 2023
    Configuration menu
    Copy the full SHA
    ec625f6 View commit details
    Browse the repository at this point in the history

  3. eth: move var to uint32 for atomic accesses 

    The TestArtificialFinality.._xxx tests are racey
because of this value; the tests want to manhandle
it while the handler goroutines (and chainsyncop calls)
are running.
This might fix it, though it feels like overkill.

Date: 2023-02-07 13:59:16-08:00
Signed-off-by: meows <[email protected]>
    @meowsbits
    meowsbits committed Feb 7, 2023
    Configuration menu
    Copy the full SHA
    5e018a5 View commit details
    Browse the repository at this point in the history