Bug 220956

Summary: Crash when remote inspecting in debug builds
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Web InspectorAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, keith_miller, mark.lam, msaboff, saam, simon.fraser, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=221107
https://bugs.webkit.org/show_bug.cgi?id=221108
Bug Depends on:    
Bug Blocks: 225794    
Attachments:
Description Flags
Patch
none
Patch none

Description Simon Fraser (smfr) 2021-01-25 15:28:22 PST
Crash when remote inspecting in debug builds
Comment 1 Simon Fraser (smfr) 2021-01-25 15:30:25 PST
Created attachment 418347 [details]
Patch
Comment 2 Devin Rousso 2021-01-25 15:40:08 PST
Comment on attachment 418347 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418347&action=review

> Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:215
> +    dispatchAsyncOnTarget([this, message = [[message copy] autorelease], strongThis = makeRef(*this)]() {

I believe that `message` isn't ever used after calling `sendMessageToTarget`.  Can we avoid creating a copy?
Comment 3 Simon Fraser (smfr) 2021-01-25 16:14:15 PST
Created attachment 418355 [details]
Patch
Comment 4 Devin Rousso 2021-01-25 16:24:38 PST
Comment on attachment 418355 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418355&action=review

r=me, nice fix!

> Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:207
> +            if (targetIdentifier)

Why was this added?  `RemoteInspector::updateTargetListing` should be able to handle `0`.
Comment 5 EWS 2021-01-25 21:09:22 PST
Committed r271876: <https://trac.webkit.org/changeset/271876>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418355 [details].
Comment 6 Radar WebKit Bug Importer 2021-01-25 21:10:14 PST
<rdar://problem/73602775>
Comment 7 BJ Burg 2021-01-25 22:17:18 PST
Comment on attachment 418355 [details]
Patch

Thanks for digging on this, Simon!
Comment 8 Joseph Pecoraro 2021-01-29 10:03:36 PST
Do we actually know why BlockPtr doesn't work well with ARC?
Comment 9 Simon Fraser (smfr) 2021-01-29 11:50:57 PST
It works fine with ARC, but not when the linker links an ARC-compiled function into a caller that is not complied with ARC.