From: Francis Dupont Date: Tue, 28 Jun 2016 17:23:30 +0000 (+0200) Subject: [fdxhook] Added gc stuff (hard again for v8), and in the doc too X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fheads%2Ffdxhook;p=thirdparty%2Fkea.git [fdxhook] Added gc stuff (hard again for v8), and in the doc too --- diff --git a/src/hooks/external/EXAMPLES b/src/hooks/external/EXAMPLES index e51d578304..fe7305d7cf 100644 --- a/src/hooks/external/EXAMPLES +++ b/src/hooks/external/EXAMPLES @@ -14,10 +14,10 @@ The 3 (python, ocaml and lua) examples are organized the same way: - a script written in the external language with a pkt4 -> int pkt4_receive function which prints its argument using toText. - Note in ocaml the sompiled script (in bytecode or native code) + Note in ocaml the compiled script (in bytecode or native code) is embedded into the dynamic shared object. - - a tests tool which loads a config with a hook-libraires entry, + - a tests tool which loads a config with a hook-libraries entry, build a DHCPv4 packet and call the pkt4_receive callout. So in running order you have: @@ -50,3 +50,10 @@ embedded language pkt4_receive function is called V 0 (next step continue) is returned (here the tests tool exits) + +I added a hook forcing a garbage collection both to check whether +shared pointers are correctly managed and because it can be useful +in production (look at NOTES for a discussion about two languages' +garbage collection (non)cooperation). BTW ocaml provides a way to +take into account dependent memory: caml_{alloc,free}_dependent_memory. +(ocaml is clearly superior but I have also clearly a bias in favor of it). diff --git a/src/hooks/external/NOTES b/src/hooks/external/NOTES index 8cd4105083..c243174d56 100644 --- a/src/hooks/external/NOTES +++ b/src/hooks/external/NOTES @@ -63,3 +63,10 @@ Answer: ocaml: int, float, string, bytes (same representation than string) lua: (number) integer, (number) float, string (include binary) +Question: garbage collection? +Answer: + boost shared pointers and python3 use reference counters. ocaml, lua + and v8 use modern tracing systems which are far better but do not +interact very well with boost shared pointers: objects referenced +from an external language by an unused value can be freed only when +the value is collected. diff --git a/src/hooks/external/inspect.cc b/src/hooks/external/inspect.cc index dd335bff88..04b0e2193d 100644 --- a/src/hooks/external/inspect.cc +++ b/src/hooks/external/inspect.cc @@ -37,34 +37,34 @@ int main() { cout << "sizeof(IntPtr ::= int*) is " << sizeof(IntPtr) * 8 << " octets\n"; cout << "sizeof(IntPtr3 ::= int***) is " << sizeof(IntPtr3) * 8 << " octets\n"; if (sizeof(IntPtr) != sizeof(IntPtr3)) { - cout << "unusual size difference between IntPtr and IntPtr3?\n"; + cout << "unusual size difference between IntPtr and IntPtr3?\n"; } if (sizeof(Four) != 4 * sizeof(IntPtr3)) { - cout << "sizeof(Four) is " << sizeof(Four) * 8 - << " octets and don't match expected " << sizeof(IntPtr3) * 32 - << " octet?\n"; + cout << "sizeof(Four) is " << sizeof(Four) * 8 + << " octets and don't match expected " << sizeof(IntPtr3) * 32 + << " octet?\n"; } Four one; Four zero; memset(&zero, 0, sizeof(Four)); if (memcmp(&zero, &one, sizeof(Four)) != 0) { - cout << "Four instance is not initialized to 0 as expected?\n"; + cout << "Four instance is not initialized to 0 as expected?\n"; } cout << "sizeof(FourPtr ::= Four*) is " << sizeof(FourPtr) * 8 << " octets\n"; cout << "sizeof(FourSharedPtr ::= shared_ptr is " << sizeof(FourSharedPtr) * 8 << " octets\n"; cout << "sizeof(Encap) is " << sizeof(Encap) * 8 << " octets\n"; if (sizeof(FourSharedPtr) != sizeof(Encap)) { - cout << "unusual size difference between Encap and FourSharedPtr\n"; + cout << "unusual size difference between Encap and FourSharedPtr\n"; } size_t size = sizeof(Encap); if (sizeof(Four) < sizeof(Encap)) { - cout << "shared pointers are very big?\n"; - size = sizeof(Four); + cout << "shared pointers are very big?\n"; + size = sizeof(Four); } Encap encap; if (memcmp(&encap, &zero, size) != 0) { - cerr << "shared pointers are not initialized to 0???\n"; - return (-1); + cerr << "shared pointers are not initialized to 0???\n"; + return (-1); } cout << "shared pointers are initialized to 0 as expected\n"; return (0); diff --git a/src/hooks/external/lua/dso.cc b/src/hooks/external/lua/dso.cc index e771e90a12..0b0950129c 100644 --- a/src/hooks/external/lua/dso.cc +++ b/src/hooks/external/lua/dso.cc @@ -205,4 +205,9 @@ int pkt4_receive(CalloutHandle& handle) { return (ret); } +// force_gc +int force_gc(CalloutHandle&) { + return (lua_gc(L, LUA_GCCOLLECT, 0)); +} + } diff --git a/src/hooks/external/lua/tests.cc b/src/hooks/external/lua/tests.cc index 5d21d3a542..c1bdf86ff2 100644 --- a/src/hooks/external/lua/tests.cc +++ b/src/hooks/external/lua/tests.cc @@ -36,6 +36,8 @@ int main() { // must be first int hi_pkt4_receive = HooksManager::registerHook("pkt4_receive"); cout << "pkt4_receive is hook#" << hi_pkt4_receive << "\n"; + int hi_force_gc = HooksManager::registerHook("force_gc"); + cout << "force_gc is hook#" << hi_force_gc << "\n"; initLogger(); @@ -140,7 +142,17 @@ int main() { cout << "pkt4_receive callout status " << co_handle->getStatus() << "\n"; co_handle->getArgument("query4", pkt); + // call the garbage collector + if (!HooksManager::calloutsPresent(hi_force_gc)) { + cout << "no callout present for force_gc\n"; + exit(0); + } + co_handle->deleteAllArguments(); + cout << "calling force_gc callout\n"; + HooksManager::callCallouts(hi_force_gc, *co_handle); + // TODO... + cout << "done...\n"; exit(0); } diff --git a/src/hooks/external/ocaml/dso.c b/src/hooks/external/ocaml/dso.c index b42ad1a94b..0a007fd96a 100644 --- a/src/hooks/external/ocaml/dso.c +++ b/src/hooks/external/ocaml/dso.c @@ -120,4 +120,11 @@ int pkt4_receive(CalloutHandle& handle) { return (Int_val(result)); } +// force_gc +extern void caml_minor_collection(); +int force_gc(CalloutHandle&) { + caml_minor_collection(); + return (0); +} + } diff --git a/src/hooks/external/ocaml/tests.c b/src/hooks/external/ocaml/tests.c index 5f0f416c31..874e3d7f92 100644 --- a/src/hooks/external/ocaml/tests.c +++ b/src/hooks/external/ocaml/tests.c @@ -49,6 +49,8 @@ int main() { // must be first int hi_pkt4_receive = HooksManager::registerHook("pkt4_receive"); cout << "pkt4_receive is hook#" << hi_pkt4_receive << "\n"; + int hi_force_gc = HooksManager::registerHook("force_gc"); + cout << "force_gc is hook#" << hi_force_gc << "\n"; initLogger(); @@ -153,7 +155,17 @@ int main() { cout << "pkt4_receive callout status " << co_handle->getStatus() << "\n"; co_handle->getArgument("query4", pkt); + // call the garbage collector + if (!HooksManager::calloutsPresent(hi_force_gc)) { + cout << "no callout present for force_gc\n"; + exit(0); + } + co_handle->deleteAllArguments(); + cout << "calling force_gc callout\n"; + HooksManager::callCallouts(hi_force_gc, *co_handle); + // TODO... + cout << "done...\n"; exit(0); } diff --git a/src/hooks/external/python/dso.cc b/src/hooks/external/python/dso.cc index 4a5acdf4f7..5eef247fc7 100644 --- a/src/hooks/external/python/dso.cc +++ b/src/hooks/external/python/dso.cc @@ -169,4 +169,10 @@ int pkt4_receive(CalloutHandle& handle) { return (result); } +// force_gc +int force_gc(CalloutHandle&) { + cout << "cpython uses reference counts\n"; + return (0); +} + } diff --git a/src/hooks/external/python/tests.cc b/src/hooks/external/python/tests.cc index 30e69da571..4609d75cbb 100644 --- a/src/hooks/external/python/tests.cc +++ b/src/hooks/external/python/tests.cc @@ -37,6 +37,8 @@ int main() { // must be first int hi_pkt4_receive = HooksManager::registerHook("pkt4_receive"); cout << "pkt4_receive is hook#" << hi_pkt4_receive << "\n"; + int hi_force_gc = HooksManager::registerHook("force_gc"); + cout << "force_gc is hook#" << hi_force_gc << "\n"; initLogger(); @@ -141,7 +143,17 @@ int main() { cout << "pkt4_receive callout status " << co_handle->getStatus() << "\n"; co_handle->getArgument("query4", pkt); + // call the garbage collector + if (!HooksManager::calloutsPresent(hi_force_gc)) { + cout << "no callout present for force_gc\n"; + exit(0); + } + co_handle->deleteAllArguments(); + cout << "calling force_gc callout\n"; + HooksManager::callCallouts(hi_force_gc, *co_handle); + // TODO... + cout << "done...\n"; exit(0); } diff --git a/src/hooks/external/v8/NOTES b/src/hooks/external/v8/NOTES index 5be6a0e092..97b80038cb 100644 --- a/src/hooks/external/v8/NOTES +++ b/src/hooks/external/v8/NOTES @@ -34,7 +34,8 @@ Manifest: v8 external API supports only raw pointers. -There is no proof the garbage collecting part is correct. BTW v8 is known -to not cleanup things (performance issue for Chrome). +v8 is known to not cleanup things (performance issue for Chrome). The documentation is nearly useless and the API unstable... + +Weak persistent external values are very hairy to manage. diff --git a/src/hooks/external/v8/dso.cc b/src/hooks/external/v8/dso.cc index cbf06d87d8..83703eb82e 100644 --- a/src/hooks/external/v8/dso.cc +++ b/src/hooks/external/v8/dso.cc @@ -285,4 +285,11 @@ int pkt4_receive(CalloutHandle& handle) { return (static_cast(ret)); } +// force_gc +int force_gc(CalloutHandle&) { + isolate_->RequestGarbageCollectionForTesting( + Isolate::GarbageCollectionType::kFullGarbageCollection); + return (0); +} + } diff --git a/src/hooks/external/v8/tests.cc b/src/hooks/external/v8/tests.cc index cdd15f03b7..a0b4e4db9d 100644 --- a/src/hooks/external/v8/tests.cc +++ b/src/hooks/external/v8/tests.cc @@ -30,7 +30,7 @@ const string config = " \"parameters\": " " { \"program\": \"kea\", " " \"script\": \"hook.js\"," - " \"flags\": \"--use_strict\" }" + " \"flags\": \"--use_strict --expose_gc\" }" " }] }"; // main routine @@ -38,6 +38,8 @@ int main() { // must be first int hi_pkt4_receive = HooksManager::registerHook("pkt4_receive"); cout << "pkt4_receive is hook#" << hi_pkt4_receive << "\n"; + int hi_force_gc = HooksManager::registerHook("force_gc"); + cout << "force_gc is hook#" << hi_force_gc << "\n"; initLogger(); @@ -142,7 +144,17 @@ int main() { cout << "pkt4_receive callout status " << co_handle->getStatus() << "\n"; co_handle->getArgument("query4", pkt); + // call the garbage collector + if (!HooksManager::calloutsPresent(hi_force_gc)) { + cout << "no callout present for force_gc\n"; + exit(0); + } + co_handle->deleteAllArguments(); + cout << "calling force_gc callout\n"; + HooksManager::callCallouts(hi_force_gc, *co_handle); + // TODO... + cout << "done...\n"; exit(0); } diff --git a/src/hooks/external/v8/voption.cc b/src/hooks/external/v8/voption.cc index 61d2959b0a..fb3de23afa 100644 --- a/src/hooks/external/v8/voption.cc +++ b/src/hooks/external/v8/voption.cc @@ -26,9 +26,9 @@ void option_finalize(const WeakCallbackData& data) { // This is a critical code to avoid memory leaks cout << "option_finalize called\n"; - Local field = - Local::Cast(data.GetValue()->GetInternalField(0)); - delete static_cast(field->Value()); + v8_option* cppobj = static_cast(data.GetParameter()); + cppobj->handle.Reset(); + delete cppobj; } // toString @@ -70,14 +70,14 @@ Local make_option(Isolate* isolate, OptionPtr opt) { } // Set the C++ part - v8_option* ccpobj(new v8_option()); - ccpobj->object = opt; - Local ptr = External::New(isolate, ccpobj); + v8_option* cppobj(new v8_option()); + cppobj->object = opt; + Local ptr = External::New(isolate, cppobj); result->SetInternalField(0, ptr); - // Show the new value to the garbage collector - Persistent gcref(isolate, result); - gcref.SetWeak(ccpobj, option_finalize); + // Set the V8 part + cppobj->handle.Reset(isolate, result); + cppobj->handle.SetWeak(cppobj, option_finalize); return (handle_scope.Escape(result)); } @@ -96,7 +96,7 @@ void init_option(Isolate* isolate) { Local tostring; if (!Function::New(isolate->GetCurrentContext(), option_tostring).ToLocal(&tostring)) { - cerr << "can't create pkt4_tostring\n"; + cerr << "can't create option_tostring\n"; } templ->Set(isolate, "toString", tostring); diff --git a/src/hooks/external/v8/voption.h b/src/hooks/external/v8/voption.h index af1ad12f71..2198b9209d 100644 --- a/src/hooks/external/v8/voption.h +++ b/src/hooks/external/v8/voption.h @@ -18,6 +18,7 @@ public: v8_option(); isc::dhcp::OptionPtr object; + ::v8::Persistent< ::v8::Object> handle; }; ::v8::Local< ::v8::Object> make_option(::v8::Isolate* isolate, diff --git a/src/hooks/external/v8/vpkt4.cc b/src/hooks/external/v8/vpkt4.cc index 66656d83e6..ebcc3de0fc 100644 --- a/src/hooks/external/v8/vpkt4.cc +++ b/src/hooks/external/v8/vpkt4.cc @@ -27,9 +27,9 @@ void pkt4_finalize(const WeakCallbackData& data) { // This is a critical code to avoid memory leaks cout << "pkt4_finalize called\n"; - Local field = - Local::Cast(data.GetValue()->GetInternalField(0)); - delete static_cast(field->Value()); + v8_pkt4* cppobj = static_cast(data.GetParameter()); + cppobj->handle.Reset(); + delete cppobj; } // toString @@ -71,14 +71,14 @@ Local make_pkt4(Isolate* isolate, Pkt4Ptr pkt) { } // Set the C++ part - v8_pkt4* ccpobj(new v8_pkt4()); - ccpobj->object = pkt; - Local ptr = External::New(isolate, ccpobj); + v8_pkt4* cppobj(new v8_pkt4()); + cppobj->object = pkt; + Local ptr = External::New(isolate, cppobj); result->SetInternalField(0, ptr); - // Show the new value to the garbage collector - Persistent gcref(isolate, result); - gcref.SetWeak(ccpobj, pkt4_finalize); + // Set the V8 part + cppobj->handle.Reset(isolate, result); + cppobj->handle.SetWeak(cppobj, pkt4_finalize); return (handle_scope.Escape(result)); } diff --git a/src/hooks/external/v8/vpkt4.h b/src/hooks/external/v8/vpkt4.h index 88a05c436a..f2cdb4ee5c 100644 --- a/src/hooks/external/v8/vpkt4.h +++ b/src/hooks/external/v8/vpkt4.h @@ -18,6 +18,7 @@ public: v8_pkt4(); isc::dhcp::Pkt4Ptr object; + ::v8::Persistent< ::v8::Object> handle; }; ::v8::Local< ::v8::Object> make_pkt4(::v8::Isolate* isolate,