Bug 229551 – PrivateClickMeasurementManager::firePendingAttributionRequests() is crashing in debug

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[youenn fablet]

See https://build-safari.apple.com/#/builders/2620/builds/100

    TestWebKitAPI.WebKit.AutoplayOnVisibilityChange
        2021-08-25 10:40:45.775 com.apple.WebKit.WebContent.Development[40235:2276300] XType: can't switch to GlobalFontRegistry.  XTFontStaticRegistry is not enabled.
        SHOULD NEVER BE REACHED
        /Volumes/Data/worker/trunk-starbravo-debug-archive/build/OpenSource/Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp(423) : auto WebKit::PrivateClickMeasurementManager::firePendingAttributionRequests()::(anonymous class)::operator()(auto &&) const [attributions:auto = WTF::Vector<WebCore::PrivateClickMeasurement, 0, WTF::CrashOnOverflow, 16>]
        1   0x11fe61009 WTFCrash
        2   0x129c9adeb WTFCrashWithInfo(int, char const*, char const*, int)
        3   0x12aaec14e auto WebKit::PrivateClickMeasurementManager::firePendingAttributionRequests()::$_22::operator()<WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> >(WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&) const
        4   0x12aaebf41 WTF::Detail::CallableWrapper<WebKit::PrivateClickMeasurementManager::firePendingAttributionRequests()::$_22, void, WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&>::call(WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)
        5   0x12abd96a7 WTF::Function<void (WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)>::operator()(WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&) const
        6   0x12ab42a10 WTF::CompletionHandler<void (WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)>::operator()(WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)
        7   0x12abda89c WebKit::WebResourceLoadStatisticsStore::allAttributedPrivateClickMeasurement(WTF::CompletionHandler<void (WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)>&&)::$_114::operator()()::'lambda0'()::operator()()
        8   0x12abda7c9 WTF::Detail::CallableWrapper<WebKit::WebResourceLoadStatisticsStore::allAttributedPrivateClickMeasurement(WTF::CompletionHandler<void (WTF::Vector<WebCore::PrivateClickMeasurement, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)>&&)::$_114::operator()()::'lambda0'(), void>::call()
        9   0x11fe8a0f2 WTF::Function<void ()>::operator()() const
        10  0x11ff0744e WTF::RunLoop::performWork()
        11  0x11ff0ad1e WTF::RunLoop::performWork(void*)
        12  0x7ff8027d0ded __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
        13  0x7ff8027d0d55 __CFRunLoopDoSource0
        14  0x7ff8027d0ad4 __CFRunLoopDoSources0
        15  0x7ff8027cf50b __CFRunLoopRun
        16  0x7ff8027ceacd CFRunLoopRunSpecific
        17  0x7ff80363366e -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
        18  0x7ff8036bda03 -[NSRunLoop(NSRunLoop) run]
        19  0x7ff80245f233 _xpc_objc_main
        20  0x7ff80245ec22 xpc_main
        21  0x12aaeedbf WebKit::XPCServiceMain(int, char const**)
        22  0x12c1cb26b WKXPCServiceMain
        23  0x1005b1ea2 main
        24  0x106c944d5

[youenn fablet]

Created attachment 436501 [details]
Patch

[youenn fablet]

<rdar://81997710>

[youenn fablet]

Bug 229550 might be explained by this crash.

[Kate Cheney]

Comment on attachment 436501 [details]
Patch

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

> Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:423
> +                // Attribution has already been sent.

I am not sure this is the right solution. I added an ASSERT here because I thought this code path was not possible. We should figure out why it is being hit instead of removing the ASSERT.

[youenn fablet]

(In reply to Kate Cheney from comment #4)
> Comment on attachment 436501 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=436501&action=review
> 
> > Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:423
> > +                // Attribution has already been sent.
> 
> I am not sure this is the right solution. I added an ASSERT here because I
> thought this code path was not possible. We should figure out why it is
> being hit instead of removing the ASSERT.

Agreed it might be fine to investigate why this assert is buggy.
But this disrupts tests so it seems good to remove this assert as soon as possible.
I am fine adding release logging instead for now.

[Kate Cheney]

Created attachment 436579 [details]
Patch

[John Wilander]

r=me

[EWS]

Committed r281697 (241047@main): <https://commits.webkit.org/241047@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 436579 [details].