Skip to content
Permalink
Browse files

Fix an add/remove race in Supplier::Preserving

The tap list was vulnerable to a situation where the `tap(...)` call
could result in a sequence of actions that would `close` that tap,
but the tap had not been added to the tap list yet. This could result
in us having a taplist containing `Mu`, and `Mu.emit` would blow up
(the call meaning to target a TapListEntry`. Even if the `Mu` insertion
were to be avoided, we'd still end up with the closed tap being put in
the list.

Resolve this fully by introducing a pair of flags to indicate if we
have added or removed the entry. Only really do the removal if we've
added it, and only really do the addition if we didn't (in spirit)
remove it.

Fixes flapping in S17-supply/supplier-preserving.t.
  • Loading branch information...
jnthn committed Nov 2, 2018
1 parent 15c16e3 commit 68d79e70fbf4c8a15109c75300ff7fcca102ad50
Showing with 24 additions and 12 deletions.
  1. +24 −12 src/core/Supply.pm6
@@ -1660,25 +1660,37 @@ my class Supplier::Preserving is Supplier {
method tap(&emit, &done, &quit, &tap) {
my $tle := TapListEntry.new(:&emit, :&done, :&quit);
my int $replay = 0;
# Since we run `tap` before adding, there's a small chance of
# a tap removal attempt happening for the add attempt. We use
# these two flags to handle that case. This is safe since we
# only ever access them under lock.
my $added := False;
my $removed := False;
my $t = Tap.new({
$!lock.protect({
my Mu $update := nqp::list();
for nqp::hllize($!tappers) -> \entry {
nqp::push($update, entry) unless entry =:= $tle;
if $added {
my Mu $update := nqp::list();
for nqp::hllize($!tappers) -> \entry {
nqp::push($update, entry) unless entry =:= $tle;
}
$!replay-done = 0 if nqp::elems($update) == 0;
$!tappers := $update;
}
$!replay-done = 0 if nqp::elems($update) == 0;
$!tappers := $update;
$removed := True;
});
});
tap($t);
$!lock.protect({
my Mu $update := nqp::isconcrete($!tappers)
?? nqp::clone($!tappers)
!! nqp::list();
nqp::push($update, $tle);
$replay = 1 if nqp::elems($update) == 1;
self!replay($tle) if $replay;
$!tappers := $update;
unless $removed {
my Mu $update := nqp::isconcrete($!tappers)
?? nqp::clone($!tappers)
!! nqp::list();
nqp::push($update, $tle);
$replay = 1 if nqp::elems($update) == 1;
self!replay($tle) if $replay;
$!tappers := $update;
}
$added := True;
});
$t
}

0 comments on commit 68d79e7

Please sign in to comment.
You can’t perform that action at this time.