Skip to content

Commit

Permalink
fix: fix memory leak of the using of dlerror (#328)
Browse files Browse the repository at this point in the history
  • Loading branch information
halajohn authored Nov 25, 2024
1 parent dff7500 commit 6d5c15b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 32 deletions.
24 changes: 24 additions & 0 deletions core/src/ten_utils/lib/sys/posix/darwin/pc/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ void *ten_module_load(const ten_string_t *name, int as_local) {
const char *err_msg = dlerror();
TEN_LOGE("Failed to dlopen %s: %s", ten_string_get_raw_str(name),
err_msg ? err_msg : "Unknown error");

// It is necessary to call `dlerror` again to clear the memory allocated for
// the previous error string by `dlerror`, to avoid the leak sanitizer
// reporting a memory leak for this error string.
dlerror();
}

return handle;
Expand All @@ -40,6 +45,13 @@ int ten_module_close(void *handle) {
const char *err_msg = dlerror();
TEN_LOGE("Failed to dlclose handle: %s",
err_msg ? err_msg : "Unknown error");

// It is necessary to call `dlerror` again to clear the memory allocated for
// the previous error string by `dlerror`, to avoid the leak sanitizer
// reporting a memory leak for this error string.
dlerror();

return -1;
}

return result;
Expand All @@ -57,6 +69,12 @@ void *ten_module_get_symbol(void *handle, const char *symbol_name) {
}

// Clear previous errors.
//
// Since `dlsym` returning `NULL` can be a normal occurrence (e.g., when the
// symbol is not defined), the return value of `dlsym` being `NULL` cannot be
// used to determine whether it is an error. Therefore, it is necessary to
// first call `dlerror` to clear any existing errors, then call `dlsym`, and
// finally call `dlerror` again to check if an error has occurred.
dlerror();

void *symbol = dlsym(handle, symbol_name);
Expand All @@ -67,6 +85,12 @@ void *ten_module_get_symbol(void *handle, const char *symbol_name) {
#if 0
TEN_LOGD("Failed to find symbol %s: %s", symbol_name, error);
#endif

// It is necessary to call `dlerror` again to clear the memory allocated for
// the previous error string by `dlerror`, to avoid the leak sanitizer
// reporting a memory leak for this error string.
dlerror();

return NULL;
}

Expand Down
27 changes: 25 additions & 2 deletions core/src/ten_utils/lib/sys/posix/linux/pc/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
//
#include "ten_utils/lib/module.h"

#include "ten_utils/lib/string.h"

#define _GNU_SOURCE
#include <dlfcn.h>

#include "include_internal/ten_utils/log/log.h"
#include "ten_utils/lib/string.h"

void *ten_module_load(const ten_string_t *name, int as_local) {
if (!name || !ten_string_check_integrity(name)) {
Expand All @@ -26,6 +25,11 @@ void *ten_module_load(const ten_string_t *name, int as_local) {
const char *err_msg = dlerror();
TEN_LOGE("Failed to dlopen %s: %s", ten_string_get_raw_str(name),
err_msg ? err_msg : "Unknown error");

// It is necessary to call `dlerror` again to clear the memory allocated for
// the previous error string by `dlerror`, to avoid the leak sanitizer
// reporting a memory leak for this error string.
dlerror();
}

return handle;
Expand All @@ -42,6 +46,13 @@ int ten_module_close(void *handle) {
const char *err_msg = dlerror();
TEN_LOGE("Failed to dlclose handle: %s",
err_msg ? err_msg : "Unknown error");

// It is necessary to call `dlerror` again to clear the memory allocated for
// the previous error string by `dlerror`, to avoid the leak sanitizer
// reporting a memory leak for this error string.
dlerror();

return -1;
}

return result;
Expand All @@ -59,6 +70,12 @@ void *ten_module_get_symbol(void *handle, const char *symbol_name) {
}

// Clear previous errors.
//
// Since `dlsym` returning `NULL` can be a normal occurrence (e.g., when the
// symbol is not defined), the return value of `dlsym` being `NULL` cannot be
// used to determine whether it is an error. Therefore, it is necessary to
// first call `dlerror` to clear any existing errors, then call `dlsym`, and
// finally call `dlerror` again to check if an error has occurred.
dlerror();

void *symbol = dlsym(handle, symbol_name);
Expand All @@ -69,6 +86,12 @@ void *ten_module_get_symbol(void *handle, const char *symbol_name) {
#if 0
TEN_LOGD("Failed to find symbol %s: %s", symbol_name, error);
#endif

// It is necessary to call `dlerror` again to clear the memory allocated for
// the previous error string by `dlerror`, to avoid the leak sanitizer
// reporting a memory leak for this error string.
dlerror();

return NULL;
}

Expand Down
30 changes: 0 additions & 30 deletions tests/ten_runtime/integration/cpp/standalone_test_cpp/test_case.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,10 @@

import subprocess
import os
import pytest
from sys import stdout
from .common import build_config


# TODO(Wei): Remove this after solving the dlsym memory leakages.
@pytest.fixture(scope="function")
def manage_env_var_for_memory_issue_detection():
original_ten_enable_memory_sanitizer = os.environ.get(
"TEN_ENABLE_MEMORY_TRACKING"
)
original_asan_options = os.environ.get("ASAN_OPTIONS")

# Set the environment variable before the test.
os.environ["TEN_ENABLE_MEMORY_TRACKING"] = "false"
os.environ["ASAN_OPTIONS"] = "detect_leaks=0"

yield

# Remove the environment variable after the test.
if original_ten_enable_memory_sanitizer is not None:
os.environ["TEN_ENABLE_MEMORY_TRACKING"] = (
original_ten_enable_memory_sanitizer
)
else:
del os.environ["TEN_ENABLE_MEMORY_TRACKING"]

if original_asan_options is not None:
os.environ["ASAN_OPTIONS"] = original_asan_options
else:
del os.environ["ASAN_OPTIONS"]


@pytest.mark.usefixtures("manage_env_var_for_memory_issue_detection")
def test_standalone_test_cpp():
base_path = os.path.dirname(os.path.abspath(__file__))
root_dir = os.path.join(base_path, "../../../../../")
Expand Down

0 comments on commit 6d5c15b

Please sign in to comment.