Bug 188491

Summary:

Break reference cycle in ErrorEvent by using JSValueInWrappedObject

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

New Bugs

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cdumez, darin, dbates, esprehn+autocc, ews-watchlist, kangil.han, kondapallykalyan, tsavell, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	darin: review+

Yusuke Suzuki

Reported 2018-08-12 13:54:16 PDT

ErrorEvent should not hold error strongly

Attachments
Patch (12.07 KB, patch) 2018-08-12 13:57 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (14.26 KB, patch) 2018-08-12 14:25 PDT, Yusuke Suzuki	darin: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2018-08-12 13:57:35 PDT

Created attachment 346988 [details] Patch

Yusuke Suzuki

Comment 2 2018-08-12 14:25:36 PDT

Created attachment 346989 [details] Patch

Darin Adler

Comment 3 2018-08-12 14:42:05 PDT

Comment on attachment 346989 [details] Patch I don’t fully understand the implications of aerialiing an object that could be changed after its serialized. Or if we are doing the right thing when there are multiple worlds involved. But that’s not specific to this patch, which is applying a pattern we already follow elsewhere.

Darin Adler

Comment 4 2018-08-12 14:43:58 PDT

Title of bug is not exactly right since it does need to hold the error “strongly” in the sense that its a reference that keeps the error object alive, just needs to do it in a way that is garbage collection compatible and doesn’t lead to reference cycles and storage leaks!

Yusuke Suzuki

Comment 5 2018-08-12 17:02:25 PDT

Committed r234789: <https://trac.webkit.org/changeset/234789>

Radar WebKit Bug Importer

Comment 6 2018-08-12 17:03:26 PDT

<rdar://problem/43210312>

Truitt Savell

Comment 7 2018-08-13 08:47:11 PDT

Looks like this revision <https://trac.webkit.org/changeset/234789> has caused build failure on Windows release https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/11156 error stdio: https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/11156/steps/compile-webkit/logs/stdio

Darin Adler

Comment 8 2018-08-13 09:47:39 PDT

Seems highly unlikely this change directly caused that build failure; maybe due to adding a new unified source file? The failure is a link error about export of a symbol affecting units tests. We should get someone to dive deeper into it. A rollout would be OK but this seems really peculiar.

Note You need to log in before you can comment on or make changes to this bug.