From a43675c1979a6055ab589e2ed09bbc29c5ba660b Mon Sep 17 00:00:00 2001 From: Alex Hultman Date: Sat, 2 Mar 2019 04:38:01 +0100 Subject: [PATCH] Clean exit, remove forcefully_free --- examples/GracefulShutdown.js | 6 ------ src/AppWrapper.h | 18 +++++++----------- src/addon.cpp | 27 ++++++++++++++++++++------- src/uws.js | 5 +++++ tests/Autobahn.js | 7 ------- tests/Hammer.js | 6 ------ uWebSockets | 2 +- 7 files changed, 33 insertions(+), 38 deletions(-) diff --git a/examples/GracefulShutdown.js b/examples/GracefulShutdown.js index be14bb3..10c3e4d 100644 --- a/examples/GracefulShutdown.js +++ b/examples/GracefulShutdown.js @@ -37,9 +37,3 @@ const app = uWS./*SSL*/App({ console.log('Failed to listen to port ' + port); } }); - -/* This is questionable */ -process.on('beforeExit', () => { - console.log('Forcefully freeing app now'); - app.forcefully_free(); -}); diff --git a/src/AppWrapper.h b/src/AppWrapper.h index de09d6e..c7cdd76 100644 --- a/src/AppWrapper.h +++ b/src/AppWrapper.h @@ -206,14 +206,6 @@ void uWS_App_listen(const FunctionCallbackInfo &args) { args.GetReturnValue().Set(args.Holder()); } -/* Mostly indended for debugging memory leaks */ -template -void uWS_App_forcefully_free(const FunctionCallbackInfo &args) { - APP *app = (APP *) args.Holder()->GetAlignedPointerFromInternalField(0); - - delete app; -} - template void uWS_App(const FunctionCallbackInfo &args) { Local appTemplate = FunctionTemplate::New(isolate); @@ -336,11 +328,15 @@ void uWS_App(const FunctionCallbackInfo &args) { appTemplate->PrototypeTemplate()->Set(String::NewFromUtf8(isolate, "ws"), FunctionTemplate::New(isolate, uWS_App_ws)); appTemplate->PrototypeTemplate()->Set(String::NewFromUtf8(isolate, "listen"), FunctionTemplate::New(isolate, uWS_App_listen)); - /* forcefully_free is unsafe for end-users to use, but nice to track memory leaks with ASAN */ - appTemplate->PrototypeTemplate()->Set(String::NewFromUtf8(isolate, "forcefully_free"), FunctionTemplate::New(isolate, uWS_App_forcefully_free)); - Local localApp = appTemplate->GetFunction()->NewInstance(isolate->GetCurrentContext()).ToLocalChecked(); localApp->SetAlignedPointerInInternalField(0, app); + /* Add this to our delete list */ + if constexpr (std::is_same::value) { + sslApps.emplace_back(app); + } else { + apps.emplace_back(app); + } + args.GetReturnValue().Set(localApp); } diff --git a/src/addon.cpp b/src/addon.cpp index 10db94e..21ffccf 100644 --- a/src/addon.cpp +++ b/src/addon.cpp @@ -25,6 +25,11 @@ using namespace v8; /* These two are definitely static */ Isolate *isolate; +bool valid = true; + +/* We hold all apps until free */ +std::vector> apps; +std::vector> sslApps; #include "Utilities.h" #include "WebSocketWrapper.h" @@ -32,10 +37,18 @@ Isolate *isolate; #include "HttpRequestWrapper.h" #include "AppWrapper.h" -/* Used for debugging */ -void print(const FunctionCallbackInfo &args) { - NativeString nativeString(isolate, args[0]); - std::cout << nativeString.getString() << std::endl; +/* Todo: Apps should be freed once the GC says so BUT ALWAYS before freeing the loop */ + +/* This has to be called in beforeExit, but exit also seems okay */ +void uWS_free(const FunctionCallbackInfo &args) { + if (valid) { + /* Freeing apps here, it could be done earlier but not sooner */ + apps.clear(); + sslApps.clear(); + /* Freeing the loop here means we give time for our timers to close, etc */ + uWS::Loop::get()->free(); + valid = false; + } } /* todo: Put this function and all inits of it in its own header */ @@ -50,13 +63,13 @@ void Main(Local exports) { /* We want this so that we can redefine process.nextTick to using the V8 native microtask queue */ isolate->SetMicrotasksPolicy(MicrotasksPolicy::kAuto); - /* Hook up our timers */ - us_loop_integrate((us_loop *) uWS::Loop::defaultLoop()); + /* Integrate with existing libuv loop, we just pass a boolean basically */ + uWS::Loop::get((void *) 1); /* uWS namespace */ exports->Set(String::NewFromUtf8(isolate, "App"), FunctionTemplate::New(isolate, uWS_App)->GetFunction()); exports->Set(String::NewFromUtf8(isolate, "SSLApp"), FunctionTemplate::New(isolate, uWS_App)->GetFunction()); - exports->Set(String::NewFromUtf8(isolate, "print"), FunctionTemplate::New(isolate, print)->GetFunction()); + exports->Set(String::NewFromUtf8(isolate, "free"), FunctionTemplate::New(isolate, uWS_free)->GetFunction()); /* Expose some µSockets functions directly under uWS namespace */ exports->Set(String::NewFromUtf8(isolate, "us_listen_socket_close"), FunctionTemplate::New(isolate, uWS_us_listen_socket_close)->GetFunction()); diff --git a/src/uws.js b/src/uws.js index 2ad88fa..c0a2cae 100644 --- a/src/uws.js +++ b/src/uws.js @@ -24,6 +24,11 @@ module.exports = (() => { f(...args); }); }; + /* You are not allowed to use the lib past here */ + process.on('exit', () => { + uWS.free(); + }); + return uWS; } catch (e) { throw new Error('This version of µWS is not compatible with your Node.js build.\n\n' + e.toString()); diff --git a/tests/Autobahn.js b/tests/Autobahn.js index 9833ce7..efbb618 100644 --- a/tests/Autobahn.js +++ b/tests/Autobahn.js @@ -71,10 +71,3 @@ listenWithSettings({ ssl: false, compression: uWS.DEDICATED_COMPRESSOR }); - -/* This is required to check for memory leaks */ -process.on('exit', () => { - apps.forEach((a) => { - a.app.forcefully_free(); - }); -}); diff --git a/tests/Hammer.js b/tests/Hammer.js index 2378da7..e3bb683 100644 --- a/tests/Hammer.js +++ b/tests/Hammer.js @@ -201,9 +201,3 @@ const app = uWS./*SSL*/App({ listenSocket = token; } }); - -/* Yes we do this crazy thing here */ -process.on('beforeExit', () => { - uWS.print('Exiting now'); - app.forcefully_free(); -}); \ No newline at end of file diff --git a/uWebSockets b/uWebSockets index 6801ab8..35e6bad 160000 --- a/uWebSockets +++ b/uWebSockets @@ -1 +1 @@ -Subproject commit 6801ab8969660a0d82b8f84f585feefabebb16fa +Subproject commit 35e6bad33b20c3e630e53b16c16546d2572bccfa