diff --git a/cpp-generator/experimental/impl_header_parser.cpp b/cpp-generator/experimental/impl_header_parser.cpp index 4ff7b4a..bf29654 100644 --- a/cpp-generator/experimental/impl_header_parser.cpp +++ b/cpp-generator/experimental/impl_header_parser.cpp @@ -1267,7 +1267,15 @@ ImplParseResult parseImplHeader(const QString& headerPath, // methods. Skip the reserved names regardless of access. static const QSet reserved = { "onContextReady", "modules", "modulePath", - "instanceId", "instancePersistencePath" + "instanceId", "instancePersistencePath", + // Teardown plumbing, same rule as onContextReady: an + // impl overriding aboutToUnload() (or calling + // unloadFinished()) is talking to the framework, not + // publishing API. Leaking either would generate a + // consumer wrapper for a lifecycle hook, and + // aboutToUnload's LogosShutdown return has no LIDL + // type anyway. + "aboutToUnload", "unloadFinished" }; if (!reserved.contains(qs(md.name))) { md.description = joinDocLines(pendingDoc).toStdString(); diff --git a/cpp-generator/experimental/lidl_gen_cdylib.cpp b/cpp-generator/experimental/lidl_gen_cdylib.cpp index eb5d383..1b4ef3e 100644 --- a/cpp-generator/experimental/lidl_gen_cdylib.cpp +++ b/cpp-generator/experimental/lidl_gen_cdylib.cpp @@ -656,6 +656,18 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module, s << "logos_module_emit_cb g_emitCb = nullptr;\n"; s << "void* g_emitUd = nullptr;\n"; s << "std::mutex g_emitMutex;\n"; + // Guarded on the protocol MINOR that introduced the teardown surface (0.5), + // exactly like the trust-root surface below. The emitted module must still + // COMPILE against an older logos-protocol, which has neither the callback + // typedef nor the two logos_module_impl.h declarations -- a module built + // against 0.4 simply has no teardown entry point, which is the same state + // as a module that never overrode the hook. Without this an older protocol + // is a hard compile error in generated code the author never sees. + s << "#if defined(LOGOS_PROTOCOL_VERSION_MINOR) && LOGOS_PROTOCOL_VERSION_MINOR >= 5\n"; + s << "logos_module_unload_done_cb g_unloadCb = nullptr;\n"; + s << "void* g_unloadUd = nullptr;\n"; + s << "std::mutex g_unloadMutex;\n"; + s << "#endif\n"; s << "std::mutex g_ctxMutex;\n"; s << "bool g_ctxStored = false;\n"; s << "std::string g_ctxPath, g_ctxId, g_ctxPersist;\n"; @@ -914,6 +926,37 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module, s << " return lp_grant_host_services(services_json);\n}\n"; s << "#endif\n\n"; + // Teardown. The callback is stored under its own mutex rather than reusing + // the emit one: it is installed on the host's thread and fired from + // whichever thread the module finishes its work on, and those are the same + // two threads the emit path already keeps apart. + s << "#if defined(LOGOS_PROTOCOL_VERSION_MINOR) && LOGOS_PROTOCOL_VERSION_MINOR >= 5\n"; + s << "void logos_module_set_unload_done_callback(logos_module_unload_done_cb cb,\n"; + s << " void* user_data)\n{\n"; + s << " std::lock_guard lock(g_unloadMutex);\n"; + s << " g_unloadCb = cb;\n"; + s << " g_unloadUd = user_data;\n"; + s << "}\n\n"; + + s << "int logos_module_about_to_unload(void)\n{\n"; + // Hand the impl a way to say "done" BEFORE asking it to unload: an impl + // that finishes inline would otherwise signal into an empty slot and the + // host would wait out the whole grace period for a module already done. + s << " _logos_codegen_::maybeSetUnloadFinished(lidlImpl(), [] {\n"; + s << " logos_module_unload_done_cb cb = nullptr;\n"; + s << " void* ud = nullptr;\n"; + s << " {\n"; + s << " std::lock_guard lock(g_unloadMutex);\n"; + s << " cb = g_unloadCb;\n"; + s << " ud = g_unloadUd;\n"; + s << " }\n"; + s << " if (cb) cb(ud);\n"; + s << " });\n"; + s << " return _logos_codegen_::maybeAboutToUnload(lidlImpl())\n"; + s << " == LogosShutdown::Asynchronous ? 1 : 0;\n"; + s << "}\n"; + s << "#endif\n\n"; + s << "const char* logos_module_get_protocol_version(void)\n{\n"; s << " return LOGOS_PROTOCOL_VERSION_STRING;\n}\n\n"; diff --git a/cpp/logos_module_context.h b/cpp/logos_module_context.h index b1b4f21..471aba8 100644 --- a/cpp/logos_module_context.h +++ b/cpp/logos_module_context.h @@ -83,6 +83,21 @@ // `LogosModuleContext::modules()` body below compiles. struct LogosModules; +// How a module answers aboutToUnload(). +// +// Synchronous — the module is already quiescent; the host may proceed to tear +// it down as soon as the call returns. +// Asynchronous — the module has work to finish first. The host waits, up to a +// bounded grace period, until the module calls unloadFinished(). +// +// Modelled on Qt Creator's IPlugin::aboutToShutdown()/ShutdownFlag, which +// solves the same problem: a plugin that cannot finish synchronously needs a +// way to say so, and a way to say when it is done. +enum class LogosShutdown { + Synchronous, + Asynchronous, +}; + class LogosModuleContext { public: virtual ~LogosModuleContext() = default; @@ -201,6 +216,18 @@ public: m_emitEventCallback = std::move(cb); } + // Framework-only — installs the callback `unloadFinished()` fires. Left + // empty outside a framework context, which is what makes unloadFinished() + // a no-op there rather than a crash. + void _logosCoreSetUnloadFinished_(std::function cb) { + m_unloadFinishedCallback = std::move(cb); + } + + // Framework-only — drives the hook. Named apart from aboutToUnload() so + // the protected override stays the only thing an author sees, and so the + // host has an entry point without making the hook itself public. + LogosShutdown _logosCoreAboutToUnload_() { return aboutToUnload(); } + protected: // Invoked from `_events_cdylib.cpp` (codegen-emitted method // bodies) to dispatch a typed event. `args` is the address of a @@ -224,6 +251,39 @@ protected: // hands the context over. virtual void onContextReady() {} + // Hook for derived impls, fired when the host is about to tear this module + // down — on an explicit unload and on application shutdown alike. Flush + // state, close handles, cancel timers here; the destructor still runs + // afterwards, but by then the framework context is gone. + // + // Return Synchronous (the default) when there is nothing to wait for. A + // module that needs to finish work returns Asynchronous and calls + // unloadFinished() when it is done — from any thread. The host waits, but + // only for a bounded grace period, after which it proceeds anyway: a hung + // module delays shutdown, it does not prevent it. Treat the deadline as + // real rather than as a courtesy. + // + // Returning Asynchronous and never calling unloadFinished() is a bug that + // costs every teardown of this module the full grace period. Returning + // Synchronous while work is still in flight is the other bug, and quieter. + // + // NOT part of the module's contract: this is framework plumbing, so the + // generator's reserved-name filter keeps it out of the derived .lidl and + // no consumer can call it. + virtual LogosShutdown aboutToUnload() { return LogosShutdown::Synchronous; } + + // Signal that the Asynchronous teardown begun in aboutToUnload() has + // finished. Safe from any thread, and safe to call when the host is not + // listening (outside a framework context, or after the grace period + // elapsed) — a no-op then rather than an error, so a module needs no + // special case for being torn down under a deadline it missed. + // + // Calling it more than once is harmless; the host acts on the first. + void unloadFinished() const { + if (m_unloadFinishedCallback) + m_unloadFinishedCallback(); + } + private: std::string m_moduleName; std::string m_modulePath; @@ -243,6 +303,9 @@ private: // when the impl is constructed outside a framework-provisioned // context, in which case `emitEventImpl_` becomes a no-op. std::function m_emitEventCallback; + // Installed by the host before it calls _logosCoreAboutToUnload_. Empty + // outside a framework context; see unloadFinished(). + std::function m_unloadFinishedCallback; }; // --------------------------------------------------------------------------- @@ -334,6 +397,37 @@ inline auto maybeSetEmitEvent(T&, std::function // Module impl didn't opt into LogosModuleContext; nothing to do. } +// Teardown, for an impl that opted into LogosModuleContext. Same tag-dispatch +// as the setters above: an impl that did not inherit the context reports +// Synchronous, which is exactly right -- it has no hook, so there is nothing to +// wait for and teardown proceeds immediately. +template +inline auto maybeSetUnloadFinished(T& impl, std::function cb) + -> std::enable_if_t> +{ + static_cast(impl)._logosCoreSetUnloadFinished_(std::move(cb)); +} + +template +inline auto maybeSetUnloadFinished(T&, std::function) + -> std::enable_if_t> +{ +} + +template +inline auto maybeAboutToUnload(T& impl) + -> std::enable_if_t, LogosShutdown> +{ + return static_cast(impl)._logosCoreAboutToUnload_(); +} + +template +inline auto maybeAboutToUnload(T&) + -> std::enable_if_t, LogosShutdown> +{ + return LogosShutdown::Synchronous; +} + } // namespace _logos_codegen_ #endif // LOGOS_MODULE_CONTEXT_H diff --git a/flake.lock b/flake.lock index e91ce32..8ad6b78 100644 --- a/flake.lock +++ b/flake.lock @@ -56,11 +56,11 @@ ] }, "locked": { - "lastModified": 1787107309, - "narHash": "sha256-oNfr0T6OrD1D56rf1brXH+9hJoJvTaPpvUdy/d62SPs=", + "lastModified": 1787267788, + "narHash": "sha256-naE4VE+F0zLsCfa/9hL7+RRRW49Pg1HisH3kOymHaKU=", "owner": "logos-co", "repo": "logos-protocol", - "rev": "f4407ff4854bdaf486182547af5b55f4a0f55229", + "rev": "0d2a3c06bfb6c5c9701da84f77afe4aab86403c6", "type": "github" }, "original": { diff --git a/tests/experimental/test_impl_header_parser.cpp b/tests/experimental/test_impl_header_parser.cpp index 47f6695..0096944 100644 --- a/tests/experimental/test_impl_header_parser.cpp +++ b/tests/experimental/test_impl_header_parser.cpp @@ -1319,3 +1319,42 @@ TEST_F(ImplHeaderParserTest, BraceInAStringLiteralIsNotAScope) ASSERT_EQ(r.module.types[0].fields.size(), 2u); EXPECT_EQ(r.module.types[0].fields[1].name, "n"); } + +// --------------------------------------------------------------------------- +// Teardown hooks stay out of the contract +// --------------------------------------------------------------------------- + +TEST_F(ImplHeaderParserTest, TeardownHooksAreNotPartOfTheContract) +{ + // aboutToUnload() and unloadFinished() are framework plumbing, exactly like + // onContextReady(). An impl that overrides one is talking to the host, not + // publishing API -- leaking either would generate a consumer wrapper for a + // lifecycle hook, and LogosShutdown has no LIDL type to return anyway. + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = dir.filePath("thing_impl.h"); + { + QFile f(hp); + ASSERT_TRUE(f.open(QIODevice::WriteOnly | QIODevice::Text)); + f.write( + "#pragma once\n" + "#include \n" + "class ThingImpl {\n" + "public:\n" + " std::string work();\n" + " void onContextReady();\n" + " LogosShutdown aboutToUnload();\n" + " void unloadFinished();\n" + "};\n"); + } + auto r = parseImplHeader(hp, "ThingImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + + QStringList names; + for (const auto& m : r.module.methods) names << QString::fromStdString(m.name); + EXPECT_TRUE(names.contains("work")) << "got: " << names.join(",").toStdString(); + EXPECT_FALSE(names.contains("aboutToUnload")) << "got: " << names.join(",").toStdString(); + EXPECT_FALSE(names.contains("unloadFinished")) << "got: " << names.join(",").toStdString(); + EXPECT_FALSE(names.contains("onContextReady")) << "got: " << names.join(",").toStdString(); +} diff --git a/tests/experimental/test_lidl_gen_cdylib.cpp b/tests/experimental/test_lidl_gen_cdylib.cpp index 4a9514e..ea909be 100644 --- a/tests/experimental/test_lidl_gen_cdylib.cpp +++ b/tests/experimental/test_lidl_gen_cdylib.cpp @@ -780,3 +780,72 @@ TEST(LidlGenCdylib, AVersionlessModuleFallsBackRatherThanEmittingEmpty) EXPECT_TRUE(implExportsFor(m).contains("std::string(\"1.0.0\")")) << implExportsFor(m).toStdString(); } + +// --- Teardown exports -------------------------------------------------------- +// +// The module ABI's unload pair is OPTIONAL by construction: the glue is +// generated alongside the module, so a cdylib built before this existed emits +// neither symbol and its consumer emits no calls. These pin what a module built +// WITH it looks like. + +TEST(LidlGenCdylib, EmitsTheOptionalTeardownExports) +{ + ModuleDecl m; + m.name = "weather_module"; + m.version = "1.0.0"; + const QString src = lidlMakeModuleImplExports(m, "SomeImpl", "some_impl.h"); + + EXPECT_TRUE(src.contains("int logos_module_about_to_unload(void)")) << src.toStdString(); + EXPECT_TRUE(src.contains("void logos_module_set_unload_done_callback(")) << src.toStdString(); +} + +TEST(LidlGenCdylib, InstallsTheCompletionCallbackBeforeAskingTheImpl) +{ + // Ordering is the whole correctness of the async path. An impl that + // finishes INLINE -- does its work and calls unloadFinished() before + // returning Asynchronous -- would otherwise signal into a slot that is + // still empty, and the host would wait out its entire grace period for a + // module that was already done. + ModuleDecl m; + m.name = "weather_module"; + const QString src = lidlMakeModuleImplExports(m, "SomeImpl", "some_impl.h"); + + const int install = src.indexOf("maybeSetUnloadFinished"); + const int ask = src.indexOf("maybeAboutToUnload"); + ASSERT_GE(install, 0) << src.toStdString(); + ASSERT_GE(ask, 0) << src.toStdString(); + EXPECT_LT(install, ask) << "completion callback installed after the unload request"; +} + +TEST(LidlGenCdylib, TeardownEmissionIsGuardedOnTheProtocolThatCarriesIt) +{ + // The teardown pair arrived in logos-protocol 0.5. A module built against + // an older protocol has neither the callback typedef nor the two + // declarations, so unguarded emission is a hard compile error in generated + // code the author never wrote and cannot see -- which is exactly what + // happened before this guard existed. Same shape as the 0.3 trust-root + // guard a few lines below it in the emitter. + ModuleDecl m; + m.name = "weather_module"; + const QString src = lidlMakeModuleImplExports(m, "SomeImpl", "some_impl.h"); + + EXPECT_TRUE(src.contains("LOGOS_PROTOCOL_VERSION_MINOR >= 5")) << src.toStdString(); + + // Both the statics and the exports must sit inside a guard: the typedef is + // what is missing on an older header, and it is named by the statics. + EXPECT_EQ(src.count("LOGOS_PROTOCOL_VERSION_MINOR >= 5"), 2) << src.toStdString(); +} + +TEST(LidlGenCdylib, TeardownGoesThroughTheSfinaeHelpersNotTheImplDirectly) +{ + // An impl that never inherited LogosModuleContext has no hook at all. The + // helpers resolve that to Synchronous at compile time; calling the impl + // directly would simply not compile for those modules. + ModuleDecl m; + m.name = "weather_module"; + const QString src = lidlMakeModuleImplExports(m, "SomeImpl", "some_impl.h"); + + EXPECT_TRUE(src.contains("_logos_codegen_::maybeAboutToUnload(lidlImpl())")) + << src.toStdString(); + EXPECT_FALSE(src.contains("lidlImpl().aboutToUnload(")) << src.toStdString(); +}