• Etienne Pierre-doray's avatar
    Reland "Reland "[wasm]: Use CancelAndDetach and barrier on BackgroundCompileJob."" · fc1d6f35
    Etienne Pierre-doray authored
    This is a reland of 064ee3c8
    
    Issue 1: WasmEngine UAF when CompilationState is destroyed
    asynchronously
    Fix: Include https://chromium-review.googlesource.com/c/v8/v8/+/2565508
    in this CL. Use OperationBarrier to keep WasmEngine alive.
    
    Issue 2: In gin, JobTask lifetime is not extended beyond
    JobHandle, thus making CancelAndDetach unusable.
    This is fixed in chromium here:
    https://chromium-review.googlesource.com/c/chromium/src/+/2566724
    
    Original change's description:
    > Reland "[wasm]: Use CancelAndDetach and barrier on BackgroundCompileJob."
    >
    > Reason for revert: Data race:
    > https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/34121
    >
    > It was assume that MockPlatform runs everything on 1 thread. However,
    > MockPlatform::PostJob previously would schedule the job through
    > TestPlatform, which eventually posts concurrent tasks, thus causing
    > data race.
    > Fix: Manually calling NewDefaultJobHandle and passing the MockPlatform
    > ensures the jobs also run sequentially.
    >
    > Additional change:
    > - CancelAndDetach is now called in ~CompilationStateImpl() to make sure
    > it's called in sequence with ScheduleCompileJobForNewUnits
    >
    > Original CL description:
    > To avoid keeping around a list of job handles, CancelAndDetach() is
    > used in CancelCompilation. Dependency on WasmEngine is handled by a
    > barrier that waits on all jobs to finish.
    >
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2498659
    > Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
    > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    > Reviewed-by: Clemens Backes <clemensb@chromium.org>
    > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    > Cr-Original-Commit-Position: refs/heads/master@{#71074}
    > Change-Id: Ie9556f7f96f6fb9a61ada0e5cbd58d4fb4a0f571
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2559137
    > Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
    > Reviewed-by: Andreas Haas <ahaas@chromium.org>
    > Cr-Commit-Position: refs/heads/master@{#71459}
    
    TBR=ulan@chromium.org
    
    Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
    Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
    Change-Id: I6175092c97fea0d5f63a97af232e2d54cccea535
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2569360
    Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
    Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#71662}
    fc1d6f35
default-job.cc 8.36 KB