feat: ensure some ten_env call timing (#264)

TEN-framework · Nov 13, 2024 · 4e84d7b · 4e84d7b
1 parent d4fd1e5
commit 4e84d7b
Show file tree

Hide file tree

Showing 12 changed files with 274 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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",
+  ]
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -24,6 +24,8 @@ class extension_tester_1 : public ten::extension_tester_t { @@
                              ten_env.stop_test();
                            }
                          });
+        ten_env.on_start_done();
       }
     };
@@ Expand Down @@