Skip to content

Commit

Permalink
app/eth2wrap: fix cancelled context race (#1378)
Browse files Browse the repository at this point in the history
Fixes a race in `eth2wrap` forkjoin code when context is cancelled before any results are returned. This resulted in `<zero>,nil` being returned which resulted in unexpected behaviour, including "divide by zero".

category: bug
ticket: #1326
  • Loading branch information
corverroos authored Nov 2, 2022
1 parent f5935e9 commit a025b4d
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 2 deletions.
12 changes: 10 additions & 2 deletions app/eth2wrap/eth2wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,9 @@ func provide[O any](ctx context.Context, clients []Client,
defer cancel()

var (
nokResp forkjoin.Result[Client, O]
zero O
nokResp forkjoin.Result[Client, O]
hasNokResp bool
zero O
)
for res := range join() {
if ctx.Err() != nil {
Expand All @@ -173,9 +174,16 @@ func provide[O any](ctx context.Context, clients []Client,
return res.Output, nil
} else {
nokResp = res
hasNokResp = true
}
}

if ctx.Err() != nil {
return zero, ctx.Err()
} else if !hasNokResp {
return zero, errors.New("bug: no forkjoin results")
}

return nokResp.Output, nokResp.Err
}

Expand Down
16 changes: 16 additions & 0 deletions app/eth2wrap/eth2wrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,19 @@ func TestErrors(t *testing.T) {
require.ErrorContains(t, err, "beacon api genesis_time: network operation error: :")
})
}

func TestCtxCancel(t *testing.T) {
for i := 0; i < 10; i++ {
ctx, cancel := context.WithCancel(context.Background())

bmock, err := beaconmock.New()
require.NoError(t, err)
eth2Cl, err := eth2wrap.NewMultiHTTP(ctx, time.Second, bmock.Address())
require.NoError(t, err)

cancel() // Cancel context before calling method.

_, err = eth2Cl.SlotDuration(ctx)
require.ErrorIs(t, err, context.Canceled)
}
}

0 comments on commit a025b4d

Please sign in to comment.