Summary: | CrashTracer: com.apple.WebKit.Networking at WebKit: WTF::Detail::CallableWrapper<WebKit::PrivateClickMeasurementManager::firePendingAttributionRequests() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kate Cheney <katherine_cheney> | ||||||
Component: | WebKit Misc. | Assignee: | Kate Cheney <katherine_cheney> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, bfulgham, cdumez, ddkilzer, webkit-bug-importer, wilander | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Kate Cheney
2021-02-19 14:32:10 PST
Created attachment 421033 [details]
Patch
Comment on attachment 421033 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=421033&action=review > Source/WebKit/ChangeLog:8 > + Could you put why you think this will fix the crash? (In reply to Alex Christensen from comment #3) > Comment on attachment 421033 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=421033&action=review > > > Source/WebKit/ChangeLog:8 > > + > > Could you put why you think this will fix the crash? Yes, will update in the next patch. TL;DR: it is crashing here: bool PrivateClickMeasurementManager::featureEnabled() const { return m_networkSession && m_networkProcess->privateClickMeasurementEnabled() && !m_sessionID.isEphemeral(); } thinking it's possible that m_networkProcess is dead when we try to check for m_networkProcess->privateClickMeasurementEnabled(). Comment on attachment 421033 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=421033&action=review > Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:57 > + , m_networkProcess(makeWeakPtr(networkProcess)) This is probably the right architecture -- we don't own the network process (i.e. we shouldn't be the thing keeping it alive), so a weakptr makes sense. > Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:-426 > - return m_networkSession && m_networkProcess->privateClickMeasurementEnabled() && !m_sessionID.isEphemeral(); It's a bummer that we can't figure out which of these things is the source of the issue. Perhaps it would be useful to early return if !m_networkSession and !m_networkProcess. (In reply to Brent Fulgham from comment #5) > Comment on attachment 421033 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=421033&action=review > > > Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:57 > > + , m_networkProcess(makeWeakPtr(networkProcess)) > > This is probably the right architecture -- we don't own the network process > (i.e. we shouldn't be the thing keeping it alive), so a weakptr makes sense. > > > Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:-426 > > - return m_networkSession && m_networkProcess->privateClickMeasurementEnabled() && !m_sessionID.isEphemeral(); > > It's a bummer that we can't figure out which of these things is the source > of the issue. Perhaps it would be useful to early return if > !m_networkSession and !m_networkProcess. Yeah, I'm in favor of splitting it into three separate conditionals just to give ourselves the information if it keeps crashing. I don't think this will fix your crash Created attachment 421043 [details]
Patch
(In reply to Alex Christensen from comment #8) > Created attachment 421043 [details] > Patch This makes so much sense. (In reply to katherine_cheney from comment #9) > (In reply to Alex Christensen from comment #8) > > Created attachment 421043 [details] > > Patch > > This makes so much sense. Yes, capturing |this| in a lambda without any kind of lifetime management rarely works out. Committed r273180: <https://commits.webkit.org/r273180> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421043 [details]. |