diff --git a/.vscode/launch.json b/.vscode/launch.json index 44481bba30..80e719c035 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -137,7 +137,7 @@ "request": "launch", "program": "${workspaceFolder}/out/linux/x64/tests/standalone/ten_runtime_smoke_test", "args": [ - "--gtest_filter=GraphTest.GroupNodeMissing2Apps" + "--gtest_filter=StandaloneTest.BasicC" ], "cwd": "${workspaceFolder}/out/linux/x64/tests/standalone/", "env": { diff --git a/core/include_internal/ten_runtime/extension/on_xxx.h b/core/include_internal/ten_runtime/extension/on_xxx.h index b731e4fb7f..1c44aa7e50 100644 --- a/core/include_internal/ten_runtime/extension/on_xxx.h +++ b/core/include_internal/ten_runtime/extension/on_xxx.h @@ -10,12 +10,12 @@ #include "ten_runtime/ten_env/ten_env.h" -TEN_RUNTIME_PRIVATE_API void ten_extension_on_configure_done(ten_env_t *self); +TEN_RUNTIME_PRIVATE_API bool ten_extension_on_configure_done(ten_env_t *self); -TEN_RUNTIME_PRIVATE_API void ten_extension_on_init_done(ten_env_t *self); +TEN_RUNTIME_PRIVATE_API bool ten_extension_on_init_done(ten_env_t *self); -TEN_RUNTIME_PRIVATE_API void ten_extension_on_start_done(ten_env_t *self); +TEN_RUNTIME_PRIVATE_API bool ten_extension_on_start_done(ten_env_t *self); -TEN_RUNTIME_PRIVATE_API void ten_extension_on_stop_done(ten_env_t *self); +TEN_RUNTIME_PRIVATE_API bool ten_extension_on_stop_done(ten_env_t *self); -TEN_RUNTIME_PRIVATE_API void ten_extension_on_deinit_done(ten_env_t *self); +TEN_RUNTIME_PRIVATE_API bool ten_extension_on_deinit_done(ten_env_t *self); diff --git a/core/src/ten_runtime/extension/extension.c b/core/src/ten_runtime/extension/extension.c index 978f673923..4193f27655 100644 --- a/core/src/ten_runtime/extension/extension.c +++ b/core/src/ten_runtime/extension/extension.c @@ -794,7 +794,7 @@ void ten_extension_on_init(ten_env_t *ten_env) { if (self->on_init) { self->on_init(self, self->ten_env); } else { - ten_extension_on_init_done(self->ten_env); + (void)ten_extension_on_init_done(self->ten_env); } } diff --git a/core/src/ten_runtime/extension/ten_env/on_xxx.c b/core/src/ten_runtime/extension/ten_env/on_xxx.c index fa01946304..4edcbdf727 100644 --- a/core/src/ten_runtime/extension/ten_env/on_xxx.c +++ b/core/src/ten_runtime/extension/ten_env/on_xxx.c @@ -74,7 +74,7 @@ static void ten_extension_adjust_and_validate_property_on_configure_done( } } -void ten_extension_on_configure_done(ten_env_t *self) { +bool ten_extension_on_configure_done(ten_env_t *self) { TEN_ASSERT(self, "Invalid argument."); TEN_ASSERT(ten_env_check_integrity(self, true), "Invalid use of ten_env %p.", self); @@ -87,6 +87,12 @@ void ten_extension_on_configure_done(ten_env_t *self) { TEN_LOGD("[%s] on_configure() done.", ten_extension_get_name(extension, true)); + if (extension->state != TEN_EXTENSION_STATE_INIT) { + TEN_LOGI("[%s] Failed to on_configure_done() because of incorrect timing.", + ten_extension_get_name(extension, true)); + return false; + } + extension->state = TEN_EXTENSION_STATE_ON_CONFIGURE_DONE; ten_extension_thread_t *extension_thread = extension->extension_thread; @@ -97,7 +103,7 @@ void ten_extension_on_configure_done(ten_env_t *self) { if (extension_thread->is_close_triggered) { // Do not proceed with the subsequent init/start flow, as the extension // thread is about to shut down. - return; + return true; } ten_error_t err; @@ -161,9 +167,11 @@ void ten_extension_on_configure_done(ten_env_t *self) { ten_extension_on_init(extension->ten_env); ten_error_deinit(&err); + + return true; } -void ten_extension_on_init_done(ten_env_t *self) { +bool ten_extension_on_init_done(ten_env_t *self) { TEN_ASSERT(self, "Invalid argument."); TEN_ASSERT(ten_env_check_integrity(self, true), "Invalid use of ten_env %p.", self); @@ -175,6 +183,13 @@ void ten_extension_on_init_done(ten_env_t *self) { TEN_LOGD("[%s] on_init() done.", ten_extension_get_name(extension, true)); + if (extension->state != TEN_EXTENSION_STATE_ON_CONFIGURE_DONE) { + // `on_init_done` can only be called at specific times. + TEN_LOGI("[%s] Failed to on_init_done() because of incorrect timing.", + ten_extension_get_name(extension, true)); + return false; + } + extension->state = TEN_EXTENSION_STATE_ON_INIT_DONE; ten_extension_thread_t *extension_thread = extension->extension_thread; @@ -183,11 +198,13 @@ void ten_extension_on_init_done(ten_env_t *self) { "Should not happen."); if (extension_thread->is_close_triggered) { - return; + return true; } // Trigger on_start of extension. ten_extension_on_start(extension); + + return true; } static void ten_extension_flush_all_pending_msgs(ten_extension_t *self) { @@ -222,7 +239,7 @@ static void ten_extension_flush_all_pending_msgs(ten_extension_t *self) { ten_list_clear(&self->pending_msgs); } -void ten_extension_on_start_done(ten_env_t *self) { +bool ten_extension_on_start_done(ten_env_t *self) { TEN_ASSERT(self, "Invalid argument."); TEN_ASSERT(ten_env_check_integrity(self, true), "Invalid use of ten_env %p.", self); @@ -234,12 +251,20 @@ void ten_extension_on_start_done(ten_env_t *self) { TEN_LOGI("[%s] on_start() done.", ten_extension_get_name(extension, true)); + if (extension->state != TEN_EXTENSION_STATE_ON_START) { + TEN_LOGI("[%s] Failed to on_start_done() because of incorrect timing.", + ten_extension_get_name(extension, true)); + return false; + } + extension->state = TEN_EXTENSION_STATE_ON_START_DONE; ten_extension_flush_all_pending_msgs(extension); + + return true; } -void ten_extension_on_stop_done(ten_env_t *self) { +bool ten_extension_on_stop_done(ten_env_t *self) { TEN_ASSERT(self, "Invalid argument."); TEN_ASSERT(ten_env_check_integrity(self, true), "Invalid use of ten_env %p.", self); @@ -251,9 +276,17 @@ void ten_extension_on_stop_done(ten_env_t *self) { TEN_LOGI("[%s] on_stop() done.", ten_extension_get_name(extension, true)); + if (extension->state != TEN_EXTENSION_STATE_ON_START_DONE) { + TEN_LOGI("[%s] Failed to on_stop_done() because of incorrect timing.", + ten_extension_get_name(extension, true)); + return false; + } + extension->state = TEN_EXTENSION_STATE_ON_STOP_DONE; ten_extension_do_pre_close_action(extension); + + return true; } static void ten_extension_thread_del_extension(ten_extension_thread_t *self, @@ -301,7 +334,7 @@ static void ten_extension_thread_on_extension_on_deinit_done( ten_extension_thread_del_extension(self, deinit_extension); } -void ten_extension_on_deinit_done(ten_env_t *self) { +bool ten_extension_on_deinit_done(ten_env_t *self) { TEN_ASSERT(self, "Invalid argument."); TEN_ASSERT(ten_env_check_integrity(self, true), "Invalid use of ten_env %p.", self); @@ -311,20 +344,26 @@ void ten_extension_on_deinit_done(ten_env_t *self) { TEN_ASSERT(ten_extension_check_integrity(extension, true), "Invalid use of extension %p.", extension); + if (extension->state != TEN_EXTENSION_STATE_ON_DEINIT) { + TEN_LOGI("[%s] Failed to on_deinit_done() because of incorrect timing.", + ten_extension_get_name(extension, true)); + return false; + } + if (!ten_list_is_empty(&self->ten_proxy_list)) { // There is still the presence of ten_env_proxy, so the closing process // cannot continue. TEN_LOGI( "[%s] Failed to on_deinit_done() because of existed ten_env_proxy.", ten_extension_get_name(extension, true)); - return; + return true; } TEN_ASSERT(extension->state >= TEN_EXTENSION_STATE_ON_DEINIT, "Should not happen."); if (extension->state == TEN_EXTENSION_STATE_ON_DEINIT_DONE) { - return; + return false; } extension->state = TEN_EXTENSION_STATE_ON_DEINIT_DONE; @@ -333,4 +372,6 @@ void ten_extension_on_deinit_done(ten_env_t *self) { ten_extension_thread_on_extension_on_deinit_done(extension->extension_thread, extension); + + return true; } diff --git a/core/src/ten_runtime/ten_env/internal/on_xxx_done.c b/core/src/ten_runtime/ten_env/internal/on_xxx_done.c index 169a7e7823..f7e7c3a7dc 100644 --- a/core/src/ten_runtime/ten_env/internal/on_xxx_done.c +++ b/core/src/ten_runtime/ten_env/internal/on_xxx_done.c @@ -29,8 +29,7 @@ bool ten_env_on_configure_done(ten_env_t *self, ten_error_t *err) { switch (self->attach_to) { case TEN_ENV_ATTACH_TO_EXTENSION: - ten_extension_on_configure_done(self); - break; + return ten_extension_on_configure_done(self); case TEN_ENV_ATTACH_TO_APP: ten_app_on_configure_done(self); @@ -58,8 +57,7 @@ bool ten_env_on_init_done(ten_env_t *self, ten_error_t *err) { switch (self->attach_to) { case TEN_ENV_ATTACH_TO_EXTENSION: - ten_extension_on_init_done(self); - break; + return ten_extension_on_init_done(self); case TEN_ENV_ATTACH_TO_EXTENSION_GROUP: ten_extension_group_on_init_done(self); @@ -98,8 +96,7 @@ bool ten_env_on_deinit_done(ten_env_t *self, TEN_UNUSED ten_error_t *err) { break; case TEN_ENV_ATTACH_TO_EXTENSION: - ten_extension_on_deinit_done(self); - break; + return ten_extension_on_deinit_done(self); case TEN_ENV_ATTACH_TO_APP: ten_app_on_deinit_done(self); @@ -210,9 +207,7 @@ bool ten_env_on_start_done(ten_env_t *self, TEN_UNUSED ten_error_t *err) { TEN_ASSERT(self->attach_to == TEN_ENV_ATTACH_TO_EXTENSION, "Should not happen."); - ten_extension_on_start_done(self); - - return true; + return ten_extension_on_start_done(self); } bool ten_env_on_stop_done(ten_env_t *self, TEN_UNUSED ten_error_t *err) { @@ -222,7 +217,5 @@ bool ten_env_on_stop_done(ten_env_t *self, TEN_UNUSED ten_error_t *err) { TEN_ASSERT(self->attach_to == TEN_ENV_ATTACH_TO_EXTENSION, "Should not happen."); - ten_extension_on_stop_done(self); - - return true; + return ten_extension_on_stop_done(self); } diff --git a/tests/ten_runtime/integration/cpp/standalone_test_cpp/default_extension_cpp/tests/basic.cc b/tests/ten_runtime/integration/cpp/standalone_test_cpp/default_extension_cpp/tests/basic.cc index 8d389f32f6..ef11a466b3 100644 --- a/tests/ten_runtime/integration/cpp/standalone_test_cpp/default_extension_cpp/tests/basic.cc +++ b/tests/ten_runtime/integration/cpp/standalone_test_cpp/default_extension_cpp/tests/basic.cc @@ -24,6 +24,8 @@ class extension_tester_1 : public ten::extension_tester_t { ten_env.stop_test(); } }); + + ten_env.on_start_done(); } }; diff --git a/tests/ten_runtime/smoke/BUILD.gn b/tests/ten_runtime/smoke/BUILD.gn index d8e0d1c733..229f7241d6 100644 --- a/tests/ten_runtime/smoke/BUILD.gn +++ b/tests/ten_runtime/smoke/BUILD.gn @@ -42,6 +42,7 @@ ten_executable("ten_runtime_smoke_test") { "notify_test", "result_conversion", "standalone_test", + "ten_env_call_timing", "video_frame_test", ] diff --git a/tests/ten_runtime/smoke/standalone_test/basic.cc b/tests/ten_runtime/smoke/standalone_test/basic.cc index 1026e662ad..c82eb5bcc0 100644 --- a/tests/ten_runtime/smoke/standalone_test/basic.cc +++ b/tests/ten_runtime/smoke/standalone_test/basic.cc @@ -53,6 +53,8 @@ class extension_tester_1 : public ten::extension_tester_t { ten_env.stop_test(); } }); + + ten_env.on_start_done(); } }; diff --git a/tests/ten_runtime/smoke/standalone_test/basic_c.cc b/tests/ten_runtime/smoke/standalone_test/basic_c.cc index d260da7538..9c037f777d 100644 --- a/tests/ten_runtime/smoke/standalone_test/basic_c.cc +++ b/tests/ten_runtime/smoke/standalone_test/basic_c.cc @@ -62,6 +62,8 @@ void ten_extension_tester_on_start(TEN_UNUSED ten_extension_tester_t *tester, if (rc) { ten_shared_ptr_destroy(hello_world_cmd); } + + ten_env_tester_on_start_done(ten_env, nullptr); } } // namespace diff --git a/tests/ten_runtime/smoke/ten_env_call_timing/BUILD.gn b/tests/ten_runtime/smoke/ten_env_call_timing/BUILD.gn new file mode 100644 index 0000000000..5476de5d39 --- /dev/null +++ b/tests/ten_runtime/smoke/ten_env_call_timing/BUILD.gn @@ -0,0 +1,24 @@ +# +# Copyright © 2024 Agora +# This file is part of TEN Framework, an open source project. +# Licensed under the Apache License, Version 2.0, with certain conditions. +# Refer to the "LICENSE" file in the root directory for more information. +# +import("//build/ten_runtime/glob.gni") +import("//build/ten_runtime/ten.gni") + +glob("ten_env_call_timing") { + file_list = all_native_files + deps = [ + "//third_party/msgpack:msgpackc", + "//third_party/nlohmann_json", + ] + include_dirs = [ + "//packages", + "//tests/ten_runtime", + ] + public_deps = [ + "//third_party/googlemock", + "//third_party/googletest", + ] +} diff --git a/tests/ten_runtime/smoke/ten_env_call_timing/on_xxx_done.cc b/tests/ten_runtime/smoke/ten_env_call_timing/on_xxx_done.cc new file mode 100644 index 0000000000..70190dcf97 --- /dev/null +++ b/tests/ten_runtime/smoke/ten_env_call_timing/on_xxx_done.cc @@ -0,0 +1,180 @@ +// +// Copyright © 2024 Agora +// This file is part of TEN Framework, an open source project. +// Licensed under the Apache License, Version 2.0, with certain conditions. +// Refer to the "LICENSE" file in the root directory for more information. +// +#include +#include + +#include "gtest/gtest.h" +#include "include_internal/ten_runtime/binding/cpp/ten.h" +#include "ten_runtime/binding/cpp/internal/ten_env.h" +#include "ten_utils/lib/thread.h" +#include "tests/common/client/cpp/msgpack_tcp.h" +#include "tests/ten_runtime/smoke/extension_test/util/binding/cpp/check.h" + +namespace { + +class test_extension : public ten::extension_t { + public: + explicit test_extension(const std::string &name) : ten::extension_t(name) {} + + void on_configure(ten::ten_env_t &ten_env) override { + bool rc = ten_env.on_init_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_start_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_stop_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_deinit_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_configure_done(); + ASSERT_EQ(rc, true); + } + + void on_init(ten::ten_env_t &ten_env) override { + bool rc = ten_env.on_start_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_stop_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_deinit_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_configure_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_init_done(); + ASSERT_EQ(rc, true); + } + + void on_cmd(ten::ten_env_t &ten_env, + std::unique_ptr cmd) override { + TEN_ENV_LOG_DEBUG(ten_env, + (std::string("on_cmd ") + cmd->get_name()).c_str()); + + if (std::string(cmd->get_name()) == "hello_world") { + auto cmd_result = ten::cmd_result_t::create(TEN_STATUS_CODE_OK); + cmd_result->set_property("detail", "hello world, too"); + ten_env.return_result(std::move(cmd_result), std::move(cmd)); + } + } + + void on_stop(ten::ten_env_t &ten_env) override { + bool rc = ten_env.on_start_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_deinit_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_configure_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_init_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_stop_done(); + ASSERT_EQ(rc, true); + } + + void on_deinit(ten::ten_env_t &ten_env) override { + bool rc = ten_env.on_start_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_stop_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_configure_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_init_done(); + ASSERT_EQ(rc, false); + + rc = ten_env.on_deinit_done(); + ASSERT_EQ(rc, true); + } +}; + +class test_app : public ten::app_t { + public: + void on_configure(ten::ten_env_t &ten_env) override { + bool rc = ten_env.init_property_from_json( + // clang-format off + R"({ + "_ten": { + "uri": "msgpack://127.0.0.1:8001/", + "log_level": 2 + } + })" + // clang-format on + , + nullptr); + ASSERT_EQ(rc, true); + + ten_env.on_configure_done(); + } +}; + +void *test_app_thread_main(TEN_UNUSED void *args) { + auto *app = new test_app(); + app->run(); + delete app; + + return nullptr; +} + +TEN_CPP_REGISTER_ADDON_AS_EXTENSION(on_xxx_done__test_extension, + test_extension); + +} // namespace + +TEST(TenEnvCallTimingTest, OnXxxDone) { // NOLINT + auto *app_thread = + ten_thread_create("app thread", test_app_thread_main, nullptr); + + // Create a client and connect to the app. + auto *client = new ten::msgpack_tcp_client_t("msgpack://127.0.0.1:8001/"); + + // Send graph. + nlohmann::json resp = client->send_json_and_recv_resp_in_json( + R"({ + "_ten": { + "type": "start_graph", + "seq_id": "55", + "nodes": [{ + "type": "extension", + "name": "test_extension", + "addon": "on_xxx_done__test_extension", + "extension_group": "test_extension_group", + "app": "msgpack://127.0.0.1:8001/" + }] + } + })"_json); + ten_test::check_status_code_is(resp, TEN_STATUS_CODE_OK); + + // Send a user-defined 'hello world' command. + resp = client->send_json_and_recv_resp_in_json( + R"({ + "_ten": { + "name": "hello_world", + "seq_id": "137", + "dest": [{ + "app": "msgpack://127.0.0.1:8001/", + "extension_group": "test_extension_group", + "extension": "test_extension" + }] + } + })"_json); + ten_test::check_result_is(resp, "137", TEN_STATUS_CODE_OK, + "hello world, too"); + + delete client; + + ten_thread_join(app_thread, -1); +} diff --git a/tests/ten_utils/unit/ten_rwlock_test.cc b/tests/ten_utils/unit/ten_rwlock_test.cc index b8cc2ff256..2fb697ed4c 100644 --- a/tests/ten_utils/unit/ten_rwlock_test.cc +++ b/tests/ten_utils/unit/ten_rwlock_test.cc @@ -320,4 +320,4 @@ TEST(RWLockTest, multi_writers_single_reader) { TEST(RWLockTest, multi_writers_multi_readers) { RunRWLockImplTest(100, TEST_THREAD_MAX, TEST_THREAD_MAX); -} \ No newline at end of file +}