Bug 220956 - Crash when remote inspecting in debug builds
Summary: Crash when remote inspecting in debug builds
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
Keywords: InRadar
Depends on:
Blocks: 225794
  Show dependency treegraph
Reported: 2021-01-25 15:28 PST by Simon Fraser (smfr)
Modified: 2021-05-13 18:05 PDT (History)
12 users (show)

See Also:

Patch (9.67 KB, patch)
2021-01-25 15:30 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (9.72 KB, patch)
2021-01-25 16:14 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
Comment 2 Devin Rousso 2021-01-25 15:40:08 PST
Comment on attachment 418347 [details]

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]
Comment 4 Devin Rousso 2021-01-25 16:24:38 PST
Comment on attachment 418355 [details]

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
Comment 7 BJ Burg 2021-01-25 22:17:18 PST
Comment on attachment 418355 [details]

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.