Skip to content
Permalink
Browse files
Web Inspector: CRASH in backend at Inspector::HeapFrontendDispatcher:…
…:garbageCollected + 552 when closing frontend/inspected page

https://bugs.webkit.org/show_bug.cgi?id=159075
<rdar://problem/26094341>

Reviewed by Timothy Hatcher.

Move the asynchronous work to a task class that can be cancelled when the
heap agent is reset, disabled or destroyed.

* inspector/agents/InspectorHeapAgent.cpp:
(Inspector::SendGarbageCollectionEventsTask::SendGarbageCollectionEventsTask):
(Inspector::SendGarbageCollectionEventsTask::addGarbageCollection):
(Inspector::SendGarbageCollectionEventsTask::reset):
(Inspector::SendGarbageCollectionEventsTask::timerFired):
Added. This holds onto GarbageCollectionData that needs to be sent asynchronously.
It uses the RunLoop variant of Timer and can queue multiple collections to be sent.
The data vector is guarded with a lock so that garbageCollected() can safely add
collection data from a non-main thread while the main thread sends out events.

(Inspector::InspectorHeapAgent::InspectorHeapAgent):
(Inspector::InspectorHeapAgent::~InspectorHeapAgent):
(Inspector::InspectorHeapAgent::disable):
Reset the task when disabling or tearing down the agent so the timer doesn't fire after destruction.

(Inspector::InspectorHeapAgent::didGarbageCollect):
Add the collection data to the task, which will dispatch an event for it asynchronously.

* inspector/agents/InspectorHeapAgent.h:


Canonical link: https://commits.webkit.org/177245@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202492 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
burg committed Jun 27, 2016
1 parent 20c6d81 commit e1c6221c078a3d06f60700f20331e6612eb3b5af
Showing with 109 additions and 11 deletions.
  1. +31 −0 Source/JavaScriptCore/ChangeLog
  2. +75 −11 Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp
  3. +3 −0 Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.h
@@ -1,3 +1,34 @@
2016-06-27 Brian Burg <bburg@apple.com>

Web Inspector: CRASH in backend at Inspector::HeapFrontendDispatcher::garbageCollected + 552 when closing frontend/inspected page
https://bugs.webkit.org/show_bug.cgi?id=159075
<rdar://problem/26094341>

Reviewed by Timothy Hatcher.

Move the asynchronous work to a task class that can be cancelled when the
heap agent is reset, disabled or destroyed.

* inspector/agents/InspectorHeapAgent.cpp:
(Inspector::SendGarbageCollectionEventsTask::SendGarbageCollectionEventsTask):
(Inspector::SendGarbageCollectionEventsTask::addGarbageCollection):
(Inspector::SendGarbageCollectionEventsTask::reset):
(Inspector::SendGarbageCollectionEventsTask::timerFired):
Added. This holds onto GarbageCollectionData that needs to be sent asynchronously.
It uses the RunLoop variant of Timer and can queue multiple collections to be sent.
The data vector is guarded with a lock so that garbageCollected() can safely add
collection data from a non-main thread while the main thread sends out events.

(Inspector::InspectorHeapAgent::InspectorHeapAgent):
(Inspector::InspectorHeapAgent::~InspectorHeapAgent):
(Inspector::InspectorHeapAgent::disable):
Reset the task when disabling or tearing down the agent so the timer doesn't fire after destruction.

(Inspector::InspectorHeapAgent::didGarbageCollect):
Add the collection data to the task, which will dispatch an event for it asynchronously.

* inspector/agents/InspectorHeapAgent.h:

2016-06-27 Michael Saboff <msaboff@apple.com>

ES6 Change: Unify handling of RegExp CharacterClassEscapes \w and \W and Word Asserts \b and \B
@@ -39,17 +39,86 @@ using namespace JSC;

namespace Inspector {

struct GarbageCollectionData {
Inspector::Protocol::Heap::GarbageCollection::Type type;
double startTime;
double endTime;
};

class SendGarbageCollectionEventsTask {
public:
SendGarbageCollectionEventsTask(HeapFrontendDispatcher&);
void addGarbageCollection(GarbageCollectionData&);
void reset();
private:
void timerFired();

HeapFrontendDispatcher& m_frontendDispatcher;
Vector<GarbageCollectionData> m_collections;
RunLoop::Timer<SendGarbageCollectionEventsTask> m_timer;
Lock m_mutex;
};

SendGarbageCollectionEventsTask::SendGarbageCollectionEventsTask(HeapFrontendDispatcher& frontendDispatcher)
: m_frontendDispatcher(frontendDispatcher)
, m_timer(RunLoop::main(), this, &SendGarbageCollectionEventsTask::timerFired)
{
}

void SendGarbageCollectionEventsTask::addGarbageCollection(GarbageCollectionData& collection)
{
{
std::lock_guard<Lock> lock(m_mutex);
m_collections.append(collection);
}

if (!m_timer.isActive())
m_timer.startOneShot(0);
}

void SendGarbageCollectionEventsTask::reset()
{
{
std::lock_guard<Lock> lock(m_mutex);
m_collections.clear();
}

m_timer.stop();
}

void SendGarbageCollectionEventsTask::timerFired()
{
Vector<GarbageCollectionData> collectionsToSend;

{
std::lock_guard<Lock> lock(m_mutex);
m_collections.swap(collectionsToSend);
}

// The timer is stopped on agent destruction, so this method will never be called after agent has been destroyed.
for (auto& collection : collectionsToSend) {
auto protocolObject = Inspector::Protocol::Heap::GarbageCollection::create()
.setType(collection.type)
.setStartTime(collection.startTime)
.setEndTime(collection.endTime)
.release();
m_frontendDispatcher.garbageCollected(WTFMove(protocolObject));
}
}

InspectorHeapAgent::InspectorHeapAgent(AgentContext& context)
: InspectorAgentBase(ASCIILiteral("Heap"))
, m_injectedScriptManager(context.injectedScriptManager)
, m_frontendDispatcher(std::make_unique<HeapFrontendDispatcher>(context.frontendRouter))
, m_backendDispatcher(HeapBackendDispatcher::create(context.backendDispatcher, this))
, m_environment(context.environment)
, m_sendGarbageCollectionEventsTask(std::make_unique<SendGarbageCollectionEventsTask>(*m_frontendDispatcher))
{
}

InspectorHeapAgent::~InspectorHeapAgent()
{
m_sendGarbageCollectionEventsTask->reset();
}

void InspectorHeapAgent::didCreateFrontendAndBackend(FrontendRouter*, BackendDispatcher*)
@@ -81,6 +150,7 @@ void InspectorHeapAgent::disable(ErrorString&)
m_enabled = false;

m_environment.vm().heap.removeObserver(this);
m_sendGarbageCollectionEventsTask->reset();

clearHeapSnapshots();
}
@@ -276,25 +346,19 @@ void InspectorHeapAgent::didGarbageCollect(HeapOperation operation)

// FIXME: Include number of bytes freed by collection.

double startTime = m_gcStartTime;
double endTime = m_environment.executionStopwatch()->elapsedTime();

// Dispatch the event asynchronously because this method may be
// called between collection and sweeping and we don't want to
// create unexpected JavaScript allocations that the Sweeper does
// not expect to encounter. JavaScript allocations could happen
// with WebKitLegacy's in process inspector which shares the same
// VM as the inspected page.

RunLoop::current().dispatch([this, startTime, endTime, operation]() {
auto collection = Inspector::Protocol::Heap::GarbageCollection::create()
.setType(protocolTypeForHeapOperation(operation))
.setStartTime(startTime)
.setEndTime(endTime)
.release();
GarbageCollectionData data;
data.type = protocolTypeForHeapOperation(operation);
data.startTime = m_gcStartTime;
data.endTime = m_environment.executionStopwatch()->elapsedTime();

m_frontendDispatcher->garbageCollected(WTFMove(collection));
});
m_sendGarbageCollectionEventsTask->addGarbageCollection(data);

m_gcStartTime = NAN;
}
@@ -37,6 +37,7 @@
namespace Inspector {

class InjectedScriptManager;
class SendGarbageCollectionEventsTask;
typedef String ErrorString;

class JS_EXPORT_PRIVATE InspectorHeapAgent : public InspectorAgentBase, public HeapBackendDispatcherHandler, public JSC::HeapObserver {
@@ -73,6 +74,8 @@ class JS_EXPORT_PRIVATE InspectorHeapAgent : public InspectorAgentBase, public H
RefPtr<HeapBackendDispatcher> m_backendDispatcher;
InspectorEnvironment& m_environment;

std::unique_ptr<SendGarbageCollectionEventsTask> m_sendGarbageCollectionEventsTask;

bool m_enabled { false };
bool m_tracking { false };
double m_gcStartTime { NAN };

0 comments on commit e1c6221

Please sign in to comment.