[Webkit-unassigned] [Bug 170390] WebAssembly: several tests added in r214504 crash when building with GCC

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 3 09:40:38 PDT 2017


https://bugs.webkit.org/show_bug.cgi?id=170390

--- Comment #11 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Saam Barati from comment #8)
> Comment on attachment 306063 [details]
> Updated patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=306063&action=review
> 
> >>> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:94
> >>> +        p.vm().promiseDeferredTimer->scheduleWorkSoon(promise, [source, promise, globalObject, plan = makeRefPtr(p)] () mutable {
> >> 
> >> Why not plan=WTFMove(plan)?
> > 
> > Because I've removed the line that created the plan to simplify it. 
> > 
> > RefPtr<Plan> plan = makeRef(p);
> > [plan = WTFMove(plan)]
> > 
> > is equivalent to 
> > 
> > [plan = makeRefPtr(p)]
> > 
> > I think this way is simpler, since we were only creating the plan to transfer it to the lambda capture (and to get the vm, but that is what caused the crash).
> 
> Gotcha. I see what's going on.
> I still think you need a local variable for the plan, otherwise, won't the
> lambda capture the outermost plan? I think that'll lead to a reference cycle.

I think it's the same, we are taking a reference of p and transferring it to the lambda.

> Even if that assignment doesn't cause this to happen, I think it's harder to
> reason about than just having a local variable.

I don't mind leaving the local variable, I just thought it was simpler this way, in one step.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170403/d15adbae/attachment.html>


More information about the webkit-unassigned mailing list