Skip to content

Commit

Permalink
fix: fix run gc cause memory leaks.
Browse files Browse the repository at this point in the history
  • Loading branch information
andycall committed Dec 20, 2021
1 parent f9c84ef commit 3f4f855
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 22 deletions.
4 changes: 2 additions & 2 deletions bridge/bindings/qjs/bom/dom_timer_coordinator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ static void handleTransientCallback(void* ptr, int32_t contextId, const char* er

handleTimerCallback(timer, errmsg);

context->timers()->removeTimeoutByID(timer->timerId());
context->timers()->removeTimeoutById(timer->timerId());
}

void DOMTimerCoordinator::installNewTimer(ExecutionContext* context, int32_t timerId, DOMTimer* timer) {
m_activeTimers[timerId] = timer;
}

void *DOMTimerCoordinator::removeTimeoutByID(int32_t timerId) {
void *DOMTimerCoordinator::removeTimeoutById(int32_t timerId) {
if (m_activeTimers.count(timerId) == 0) return nullptr;
DOMTimer* timer = m_activeTimers[timerId];

Expand Down
3 changes: 1 addition & 2 deletions bridge/bindings/qjs/bom/dom_timer_coordinator.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ class DOMTimerCoordinator {

// Removes and disposes the timer with the specified ID, if any. This may
// destroy the timer.
void* removeTimeoutByID(int32_t timerId);

void* removeTimeoutById(int32_t timerId);
DOMTimer* getTimerById(int32_t timerId);

void trace(JSRuntime* rt, JSValueConst val, JS_MarkFunc* mark_func);
Expand Down
8 changes: 4 additions & 4 deletions bridge/bindings/qjs/bom/timer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ static void handleTimerCallback(DOMTimer *timer, const char* errmsg) {
return;
}

if (context->timers()->getTimerById(timer->timerId()) == nullptr) return;

// Trigger timer callbacks.
timer->fire();

Expand All @@ -74,7 +76,7 @@ static void handleTransientCallback(void* ptr, int32_t contextId, const char* er

handleTimerCallback(timer, errmsg);

context->timers()->removeTimeoutByID(timer->timerId());
context->timers()->removeTimeoutById(timer->timerId());
}

static void handlePersistentCallback(void* ptr, int32_t contextId, const char* errmsg) {
Expand Down Expand Up @@ -136,8 +138,6 @@ static JSValue setTimeout(JSContext* ctx, JSValueConst this_val, int argc, JSVal

context->timers()->installNewTimer(context, timerId, timer);

KRAKEN_LOG(VERBOSE) << "SetTimeout " << JS_VALUE_GET_PTR(callbackValue) << " id " << timerId;

// `-1` represents ffi error occurred.
if (timerId == -1) {
return JS_ThrowTypeError(ctx, "Failed to execute 'setTimeout': dart method (setTimeout) execute failed");
Expand Down Expand Up @@ -213,7 +213,7 @@ static JSValue clearTimeout(JSContext* ctx, JSValueConst this_val, int argc, JSV
TEST_clearTimeout(timer);
#endif

context->timers()->removeTimeoutByID(id);
context->timers()->removeTimeoutById(id);
return JS_NULL;
}

Expand Down
4 changes: 4 additions & 0 deletions bridge/bindings/qjs/bom/window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,13 @@ JSValue Window::open(JSContext* ctx, JSValue this_val, int argc, JSValue* argv)
return window->callNativeMethods("open", 1, arguments);
}
JSValue Window::scrollTo(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
#if FLUTTER_BACKEND
auto window = static_cast<WindowInstance*>(JS_GetOpaque(this_val, Window::classId()));
NativeValue arguments[] = {jsValueToNativeValue(ctx, argv[0]), jsValueToNativeValue(ctx, argv[1])};
return window->callNativeMethods("scroll", 2, arguments);
#else
return JS_UNDEFINED;
#endif
}
JSValue Window::scrollBy(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto window = static_cast<WindowInstance*>(JS_GetOpaque(this_val, Window::classId()));
Expand Down
7 changes: 7 additions & 0 deletions bridge/bindings/qjs/dom/event_target.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,15 @@ bool EventTargetInstance::internalDispatchEvent(EventInstance* eventInstance) {
auto _dispatchEvent = [&eventInstance, this](JSValue& handler) {
if (eventInstance->propagationImmediatelyStopped())
return;

/* 'handler' might be destroyed when calling itself (if it frees the
handler), so must take extra care */
JS_DupValue(m_ctx, handler);

// The third params `thisObject` to null equals global object.
JSValue returnedValue = JS_Call(m_ctx, handler, JS_NULL, 1, &eventInstance->jsObject);

JS_FreeValue(m_ctx, handler);
m_context->handleException(&returnedValue);
m_context->drainPendingPromiseJobs();
JS_FreeValue(m_ctx, returnedValue);
Expand Down
4 changes: 0 additions & 4 deletions bridge/bindings/qjs/dom/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,6 @@ IMPL_PROPERTY_SETTER(Node, textContent)(JSContext* ctx, JSValue this_val, int ar
nodeInstance->internalSetTextContent(argv[0]);
return JS_NULL;
}
IMPL_PROPERTY_GETTER(Node, childNodes)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto* nodeInstance = static_cast<NodeInstance*>(JS_GetOpaque(this_val, Node::classId(this_val)));
// return JS_DupValue(ctx, nodeInstance->childNodes);
}

bool NodeInstance::isConnected() {
bool _isConnected = this == document();
Expand Down
1 change: 0 additions & 1 deletion bridge/bindings/qjs/dom/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ class Node : public EventTarget {
DEFINE_PROTOTYPE_READONLY_PROPERTY(previousSibling);
DEFINE_PROTOTYPE_READONLY_PROPERTY(nextSibling);
DEFINE_PROTOTYPE_READONLY_PROPERTY(nodeType);
DEFINE_PROTOTYPE_READONLY_PROPERTY(childNodes);

DEFINE_PROTOTYPE_FUNCTION(cloneNode, 1);
DEFINE_PROTOTYPE_FUNCTION(appendChild, 1);
Expand Down
7 changes: 0 additions & 7 deletions bridge/page_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,6 @@ static JSValue simulateInputText(JSContext* ctx, JSValueConst this_val, int argc
return JS_NULL;
};

static JSValue runGC(JSContext* ctx, JSValueConst this_val, int argc, JSValueConst* argv) {
auto* context = static_cast<binding::qjs::ExecutionContext*>(JS_GetContextOpaque(ctx));
JS_RunGC(context->runtime());
return JS_NULL;
}

static JSValue parseHTML(JSContext* ctx, JSValueConst this_val, int argc, JSValueConst* argv) {
auto* context = static_cast<binding::qjs::ExecutionContext*>(JS_GetContextOpaque(ctx));

Expand Down Expand Up @@ -239,7 +233,6 @@ KrakenPageTest::KrakenPageTest(KrakenPage* bridge) : m_page(bridge), m_page_cont
QJS_GLOBAL_BINDING_FUNCTION(m_page_context, simulatePointer, "__kraken_simulate_pointer__", 1);
QJS_GLOBAL_BINDING_FUNCTION(m_page_context, simulateInputText, "__kraken_simulate_inputtext__", 1);
QJS_GLOBAL_BINDING_FUNCTION(m_page_context, triggerGlobalError, "__kraken_trigger_global_error__", 0);
QJS_GLOBAL_BINDING_FUNCTION(m_page_context, runGC, "__kraken_run_gc__", 0);
QJS_GLOBAL_BINDING_FUNCTION(m_page_context, parseHTML, "__kraken_parse_html__", 1);

initKrakenTestFramework(bridge);
Expand Down
1 change: 0 additions & 1 deletion bridge/polyfill/src/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ class JasmineTracker {
clearAllTimer();
resetDocumentElement();
kraken.methodChannel.clearMethodCallHandler();
__kraken_run_gc__();
}
specStarted(result) {
}
Expand Down
17 changes: 16 additions & 1 deletion bridge/test/kraken_test_env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
*/

#include "kraken_test_env.h"
#include "bindings/qjs/dom/event_target.h"
#include <sys/time.h>
#include "kraken_bridge_test.h"
#include <vector>

#if defined(__linux__) || defined(__APPLE__)
Expand Down Expand Up @@ -87,8 +89,8 @@ static bool jsPool(ExecutionContext *context) {
/* the timer expired */
func = th->func;
th->func = nullptr;
unlink_timer(ts, th);
func(th->timer, th->contextId, nullptr);
unlink_timer(ts, th);
return false;
}
}
Expand All @@ -104,3 +106,16 @@ void TEST_runLoop(ExecutionContext *context) {
break;
}
}

void TEST_dispatchEvent(EventTargetInstance* eventTarget, const std::string type) {
NativeEventTarget *nativeEventTarget = new NativeEventTarget(eventTarget);
auto nativeEventType = stringToNativeString(type);
NativeEvent* nativeEvent = new NativeEvent();
nativeEvent->type = nativeEventType.get();

RawEvent rawEvent{
reinterpret_cast<uint64_t*>(nativeEvent)
};

NativeEventTarget::dispatchEventImpl(nativeEventTarget, nativeEventType.get(), &rawEvent, false);
}
2 changes: 2 additions & 0 deletions bridge/test/kraken_test_env.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define KRAKENBRIDGE_TEST_KRAKEN_TEST_ENV_H_

#include "bindings/qjs/bom/timer.h"
#include "bindings/qjs/dom/event_target.h"
#include "include/dart_methods.h"

using namespace kraken::binding::qjs;
Expand All @@ -15,5 +16,6 @@ void TEST_init(ExecutionContext *context);
int32_t TEST_setTimeout(DOMTimer *timer, int32_t contextId, AsyncCallback callback, int32_t timeout);
void TEST_clearTimeout(DOMTimer* timer);
void TEST_runLoop(ExecutionContext *context);
void TEST_dispatchEvent(EventTargetInstance* eventTarget, const std::string type);

#endif // KRAKENBRIDGE_TEST_KRAKEN_TEST_ENV_H_
49 changes: 49 additions & 0 deletions bridge/test/run_integration_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright (C) 2021 Alibaba Inc. All rights reserved.
* Author: Kraken Team.
*/

#include "gtest/gtest.h"
#include "page.h"
#include "kraken_test_env.h"
#include "kraken_bridge_test.h"
#include <fstream>

std::string readTestSpec() {
std::string filepath = std::string(SPEC_FILE_PATH) + "/../integration_tests/.specs/core.build.js";

std::ifstream file;
file.open(filepath);

std::string content;
if (file.is_open()) {
std::string line;
while (std::getline(file, line)) {
content += line + "\n";
}
file.close();
}

return content;
}

TEST(IntegrationTest, runSpecs) {
initJSPagePool(1);
initTestFramework(0);

auto* bridge = static_cast<kraken::KrakenPage*>(getPage(0));

auto& context = bridge->getContext();

TEST_init(context.get());

std::string code = readTestSpec();
bridge->evaluateScript(code.c_str(), code.size(), "vm://", 0);

executeTest(context->getContextId(), [](int32_t contextId, NativeString* status) -> void* {
KRAKEN_LOG(VERBOSE) << "done";
});

TEST_runLoop(context.get());
disposePage(0);
}
3 changes: 3 additions & 0 deletions bridge/test/test.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ elseif($ENV{KRAKEN_JS_ENGINE} MATCHES "quickjs")
./bindings/qjs/bom/window_test.cc
./bindings/qjs/dom/custom_event_test.cc
./bindings/qjs/module_manager_test.cc
# ./test/run_integration_test.cc # Only for debug usage.
)

### kraken_unit_test executable
Expand All @@ -48,6 +49,8 @@ elseif($ENV{KRAKEN_JS_ENGINE} MATCHES "quickjs")

target_compile_definitions(kraken_unit_test PUBLIC -DFLUTTER_BACKEND=0)
target_compile_definitions(kraken_unit_test PUBLIC -DUNIT_TEST=1)
target_compile_definitions(kraken_unit_test PUBLIC -DSPEC_FILE_PATH="${CMAKE_CURRENT_SOURCE_DIR}")

target_compile_definitions(kraken_static PUBLIC -DFLUTTER_BACKEND=1)
if (DEFINED ENV{LIBRARY_OUTPUT_DIR})
set_target_properties(kraken_unit_test
Expand Down

0 comments on commit 3f4f855

Please sign in to comment.