Skip to content

Commit e1d780d

Browse files
patrickanglerobert-jenner
patrickangle
authored and
robert-jenner
committed
Web Inspector: Deeply nested console logging can cause ConsoleMessage to be destroyed while we are still trying to log it
https://bugs.webkit.org/show_bug.cgi?id=256932 rdar://108063640 Reviewed by Mark Lam. InspectorConsoleAgent has been incorrectly managing the lifetime of each ConsoleMessage by moving messages into a vector of messages before we have passed those messages on to the frontend. This means that a console message that causes another console message to be logged may eventually cause our original message to be dropped from the vector if this occurs deeply enough. At that point, when we unwind back to the original message, it has been destroyed while we are trying to send it to the frontend, which results in a bad time because we are still inside a function on the ConsoleMessage that tries to access its own member variables like m_arguments, and fails because the message object itself has been destroyed, taking its members with it. This also fixes a subtle message inversion in the backlog of messages and what is logged to the console when Web Inspector is open, since the message is sent to the frontend after all other processing, but we were previously adding the message to the vector of past messages before that processing (which may in turn cause more logging before the actual logging we called in to perform) to occur. A second minor issue is that ScriptCallStack is incorrectly asserting in its constructor, which preventing investigation of this issue initially. The call stack should be less than __or equal to__ the max call stack, not just less than. * Source/JavaScriptCore/inspector/ConsoleMessage.cpp: (Inspector::ConsoleMessage::addToFrontend): * Source/JavaScriptCore/inspector/InjectedScript.cpp: (Inspector::InjectedScript::wrapObject const): (Inspector::InjectedScript::wrapTable const): * Source/JavaScriptCore/inspector/ScriptCallStack.cpp: (Inspector::ScriptCallStack::ScriptCallStack): * Source/JavaScriptCore/inspector/agents/InspectorConsoleAgent.cpp: (Inspector::InspectorConsoleAgent::addConsoleMessage): * Source/WebCore/page/PageConsoleClient.cpp: (WebCore::PageConsoleClient::messageWithTypeAndLevel): Originally-landed-as: 259548.777@safari-7615-branch (e74d4b0). rdar://108063640 Canonical link: https://commits.webkit.org/266438@main
1 parent 0f8aafd commit e1d780d

2 files changed

Lines changed: 3 additions & 4 deletions

File tree

‎Source/JavaScriptCore/inspector/ScriptCallStack.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ ScriptCallStack::ScriptCallStack(Vector<ScriptCallFrame>&& frames, bool truncate
5353
, m_truncated(truncated)
5454
, m_parentStackTrace(parentStackTrace)
5555
{
56-
ASSERT(m_frames.size() < maxCallStackSizeToCapture);
56+
ASSERT(m_frames.size() <= maxCallStackSizeToCapture);
5757
}
5858

5959
ScriptCallStack::~ScriptCallStack()

‎Source/JavaScriptCore/inspector/agents/InspectorConsoleAgent.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,15 +252,14 @@ void InspectorConsoleAgent::addConsoleMessage(std::unique_ptr<ConsoleMessage> co
252252
if (m_enabled)
253253
previousMessage->updateRepeatCountInConsole(*m_frontendDispatcher);
254254
} else {
255-
ConsoleMessage* newMessage = consoleMessage.get();
256-
m_consoleMessages.append(WTFMove(consoleMessage));
257255
if (m_enabled) {
258256
auto generatePreview = !m_isAddingMessageToFrontend;
259257
SetForScope isAddingMessageToFrontend(m_isAddingMessageToFrontend, true);
260258

261-
newMessage->addToFrontend(*m_frontendDispatcher, m_injectedScriptManager, generatePreview);
259+
consoleMessage->addToFrontend(*m_frontendDispatcher, m_injectedScriptManager, generatePreview);
262260
}
263261

262+
m_consoleMessages.append(WTFMove(consoleMessage));
264263
if (m_consoleMessages.size() >= maximumConsoleMessages) {
265264
m_expiredConsoleMessageCount += expireConsoleMessagesStep;
266265
m_consoleMessages.remove(0, expireConsoleMessagesStep);

0 commit comments

Comments
 (0)