Bug 180590 – Web Inspector: CRASH at InspectorConsoleAgent::enable when iterating mutable list of buffered console messages

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

CRASH at InspectorConsoleAgent::enable when iterating mutable list of buffered console messages

> void InspectorConsoleAgent::enable()
> {
>     ...
>     size_t messageCount = m_consoleMessages.size();
>     for (size_t i = 0; i < messageCount; ++i)
>         m_consoleMessages[i]->addToFrontend(*m_frontendDispatcher, m_injectedScriptManager, false);
> }

Saw a crash in the debugger at this point:

  * Original messagesCount was 96
  * i was 93 and the m_consoleMessages.size() was 93

This is likely only possible if when logging a console message causes another console message to happen, but we shouldn't iterate a list that can mutate (m_consoleMessages).

[Joseph Pecoraro]

<rdar://problem/35882767>

[Joseph Pecoraro]

Created attachment 328839 [details]
[PATCH] Proposed Fix

[Mark Lam]

Comment on attachment 328839 [details]
[PATCH] Proposed Fix

r=me

[WebKit Commit Bot]

Comment on attachment 328839 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 328839

Committed r225693: <https://trac.webkit.org/changeset/225693>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 328839 [details]
[PATCH] Proposed Fix

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

> Source/JavaScriptCore/inspector/agents/InspectorConsoleAgent.cpp:90
> +    Vector<std::unique_ptr<ConsoleMessage>> messages;
> +    m_consoleMessages.swap(messages);
> +
> +    for (size_t i = 0; i < messages.size(); ++i)
> +        messages[i]->addToFrontend(*m_frontendDispatcher, m_injectedScriptManager, false);

The above is how we used to write code like this before we had move semantics. Now we can do better:

    auto messages = WTFMove(m_consoleMessage);
    for (auto message : messages)
        message->addToFrontend(*m_frontendDispatcher, m_injectedScriptManager, false);