Skip to content

Commit

Permalink
fix: avoid native addon loader files to trigger other addon loadres t…
Browse files Browse the repository at this point in the history
…o load
  • Loading branch information
halajohn committed Jan 7, 2025
1 parent eee1896 commit ed3939f
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 34 deletions.
7 changes: 3 additions & 4 deletions core/include_internal/ten_runtime/addon/addon_autoload.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <stdbool.h>

#include "include_internal/ten_runtime/addon/addon.h"
#include "include_internal/ten_runtime/addon/addon_manager.h"
#include "ten_utils/lib/error.h"

typedef struct ten_app_t ten_app_t;
Expand All @@ -25,9 +24,9 @@ TEN_RUNTIME_PRIVATE_API bool ten_addon_load_all_from_ten_package_base_dirs(
ten_list_t *ten_package_base_dirs, ten_error_t *err);

TEN_RUNTIME_PRIVATE_API bool
ten_addon_try_load_specific_addon_from_app_base_dir(const char *app_base_dir,
TEN_ADDON_TYPE addon_type,
const char *addon_name);
ten_addon_try_load_specific_addon_using_native_addon_loader(
const char *app_base_dir, TEN_ADDON_TYPE addon_type,
const char *addon_name);

TEN_RUNTIME_PRIVATE_API bool
ten_addon_try_load_specific_addon_using_all_addon_loaders(
Expand Down
15 changes: 12 additions & 3 deletions core/src/ten_runtime/addon/addon.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,21 +187,30 @@ bool ten_addon_create_instance_async(ten_env_t *ten_env,
false),
"Should not happen.");

if (!ten_addon_try_load_specific_addon_from_app_base_dir(
// First, try to load it using the built-in native addon loader (i.e.,
// `dlopen`).
if (!ten_addon_try_load_specific_addon_using_native_addon_loader(
ten_string_get_raw_str(&app->base_dir), addon_type, addon_name)) {
return false;
TEN_LOGI(
"Unable to load addon %s:%s using native addon loader, will try "
"other methods.",
ten_addon_type_to_string(addon_type), addon_name);
}

if (!ten_addon_try_load_specific_addon_using_all_addon_loaders(
addon_type, addon_name)) {
return false;
TEN_LOGI("Unable to load addon %s:%s using all installed addon loaders.",
ten_addon_type_to_string(addon_type), addon_name);
}

// Find again.
addon_host = ten_addon_host_find(addon_type, addon_name);
}

if (!addon_host) {
TEN_LOGE(
"Failed to find addon %s:%s, please make sure the addon is installed.",
ten_addon_type_to_string(addon_type), addon_name);
return false;
}

Expand Down
18 changes: 12 additions & 6 deletions core/src/ten_runtime/addon/addon_autoload.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@
*/
#if defined(OS_MACOS) || defined(OS_LINUX) || defined(OS_WINDOWS)

static void load_all_dynamic_libraries_under_path(const char *path) {
static bool load_all_dynamic_libraries_under_path(const char *path) {
TEN_ASSERT(path, "Invalid argument.");

bool load_at_least_one = false;

ten_dir_fd_t *dir = NULL;
ten_path_itor_t *itor = NULL;
ten_string_t *file_path = NULL;
Expand Down Expand Up @@ -103,6 +105,8 @@ static void load_all_dynamic_libraries_under_path(const char *path) {
goto continue_loop;
}

load_at_least_one = true;

TEN_LOGD("Loaded module: %s", ten_string_get_raw_str(file_path));

continue_loop:
Expand All @@ -123,6 +127,8 @@ static void load_all_dynamic_libraries_under_path(const char *path) {
if (dir) {
ten_path_close_dir(dir);
}

return load_at_least_one;
}

static void ten_addon_load_from_base_dir(const char *path) {
Expand Down Expand Up @@ -315,7 +321,7 @@ bool ten_addon_load_all_from_ten_package_base_dirs(
return success;
}

static bool ten_addon_load_specific_addon_from_app_base_dir(
static bool ten_addon_load_specific_addon_using_native_addon_loader(
const char *app_base_dir, TEN_ADDON_TYPE addon_type, const char *addon_name,
ten_error_t *err) {
TEN_ASSERT(app_base_dir, "Invalid argument.");
Expand Down Expand Up @@ -346,7 +352,7 @@ static bool ten_addon_load_specific_addon_from_app_base_dir(
}

// Load the library from the 'lib/' directory.
load_all_dynamic_libraries_under_path(
success = load_all_dynamic_libraries_under_path(
ten_string_get_raw_str(&addon_lib_folder_path));

done:
Expand Down Expand Up @@ -380,11 +386,11 @@ static bool ten_addon_register_specific_addon(
return success;
}

bool ten_addon_try_load_specific_addon_from_app_base_dir(
bool ten_addon_try_load_specific_addon_using_native_addon_loader(
const char *app_base_dir, TEN_ADDON_TYPE addon_type,
const char *addon_name) {
if (ten_addon_load_specific_addon_from_app_base_dir(app_base_dir, addon_type,
addon_name, NULL)) {
if (ten_addon_load_specific_addon_using_native_addon_loader(
app_base_dir, addon_type, addon_name, NULL)) {
if (!ten_addon_register_specific_addon(addon_type, addon_name, NULL,
NULL)) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion core/src/ten_runtime/addon/addon_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ bool ten_addon_manager_register_specific_addon(ten_addon_manager_t *self,
} else {
TEN_ASSERT(!found_node, "Should not happen.");

TEN_LOGW("Addon '%s:%s' not found in registry.",
TEN_LOGI("Unable to find '%s:%s' in registry.",
ten_addon_type_to_string(addon_type), addon_name);
}

Expand Down
2 changes: 1 addition & 1 deletion core/ten_gn
17 changes: 17 additions & 0 deletions tests/ten_runtime/integration/common/fs_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,23 @@ def copy_tree(src_path: str, dst_path: str, rm_dst=False) -> None:
if not os.path.isdir(src_path):
raise Exception(src_path + " is not a directory.")

# Check if dst_path exists and is a directory.
if not os.path.exists(dst_path):
try:
os.makedirs(dst_path)
except Exception as exc:
raise Exception(
inspect.cleandoc(
f"""Failed to create destination directory:
{dst_path}
Exception: {exc}"""
)
)
elif not os.path.isdir(dst_path):
raise Exception(
f"Destination path '{dst_path}' exists and is not a directory."
)

try:
if rm_dst:
remove_tree(dst_path)
Expand Down
11 changes: 10 additions & 1 deletion tests/ten_runtime/integration/python/send_cmd_python/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,16 @@ ten_package_test_prepare_app("send_cmd_python_app") {
"send_cmd_python_app/property.json",
]

replace_files_after_install_all = [ "send_cmd_python_app/ten_packages/extension/default_extension_python/extension.py" ]
replace_files_after_install_all = [
"send_cmd_python_app/ten_packages/extension/default_extension_python/extension.py",

# Simulate a scenario where there is a `lib/` directory under the directory
# of a Python extension addon, and test the case where the native addon
# loader (`dlopen`) fails to load. Ensure that this does not block other
# installed addon loaders (such as the Python addon loader) from continuing
# to load the addon.
"send_cmd_python_app/ten_packages/extension/default_extension_python/lib",
]

if (ten_enable_ten_manager) {
deps = [
Expand Down

This file was deleted.

0 comments on commit ed3939f

Please sign in to comment.