Skip to content

Commit

Permalink
feat: enable memory leak checking for TEN go app and fix all leaks (#690
Browse files Browse the repository at this point in the history 
)

* feat: enable memory leak checking for TEN go app and fix all leaks

* fix: refine codes

* fix: refine codes

* fix: refine codes

* fix: refine codes
  • Loading branch information
@halajohn
halajohn authored Feb 9, 2025
1 parent 2a26c14 commit a537e7f
Show file tree
Hide file tree
Showing 94 changed files with 1,861 additions and 264 deletions.
64 changes: 54 additions & 10 deletions build/ten_runtime/feature/prepare_integration_test_resources.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,40 @@ def __init__(self):
self.src_app: str
self.src_app_language: str
self.generated_app_src_root_dir_name: str
self.replace_paths_after_install_app: list[str]
self.replace_paths_after_install_all: list[str]
self.replace_paths_after_install_app: list[list[str]]
self.replace_paths_after_install_all: list[list[str]]


def dump_integration_test_preparation_info_json(args: ArgumentInfo):
def extract_dest(item):
if isinstance(item, list):
if len(item) >= 2:
return item[1]
elif len(item) == 1:
return item[0]
return None

info = {
"test_case_src_dir": args.test_case_src_dir,
"src_app": args.src_app,
"src_app_language": args.src_app_language,
"generated_app_src_root_dir_name": args.generated_app_src_root_dir_name,
"replace_paths_after_install_app": args.replace_paths_after_install_app,
"replace_paths_after_install_all": args.replace_paths_after_install_all,
"replace_paths_after_install_app": (
[
extract_dest(item)
for item in args.replace_paths_after_install_app
]
if args.replace_paths_after_install_app
else []
),
"replace_paths_after_install_all": (
[
extract_dest(item)
for item in args.replace_paths_after_install_all
]
if args.replace_paths_after_install_all
else []
),
}

resource_dir = os.path.join(
Expand All @@ -48,7 +70,7 @@ def dump_integration_test_preparation_info_json(args: ArgumentInfo):


def copy_replacement_files(
args: ArgumentInfo, replaced_files: list[str], dest_dir: str
args: ArgumentInfo, replaced_files: list[list[str]], dest_dir: str
):
if replaced_files is None:
return
Expand All @@ -60,9 +82,25 @@ def copy_replacement_files(
dest_dir,
)

for file in replaced_files:
src_file = os.path.join(args.test_case_src_dir, file)
dst_file = os.path.join(out_dir, file)
for entry in replaced_files:
if isinstance(entry, list):
if len(entry) == 1:
src_rel = entry[0]
dst_rel = entry[0]
elif len(entry) == 2:
src_rel, dst_rel = entry
else:
print(
f"Invalid replacement specification: {entry}."
"Must be one or two strings."
)
continue
else:
src_rel = entry
dst_rel = entry

src_file = os.path.join(args.test_case_src_dir, src_rel)
dst_file = os.path.join(out_dir, dst_rel)
if not os.path.exists(src_file):
print(f"File {src_file} does not exist")
continue
Expand Down Expand Up @@ -114,15 +152,21 @@ def copy_replacement_files(
parser.add_argument(
"--replace-paths-after-install-app",
type=str,
nargs="+",
action="append",
help="List of files to replace after installing app",
help="List of files to replace after installing app. "
"Each occurrence can be one string (source path) or two strings "
"(source and destination paths).",
)

parser.add_argument(
"--replace-paths-after-install-all",
type=str,
nargs="+",
action="append",
help="List of files to replace after installing all",
help="List of files to replace after installing all. "
"Each occurrence can be one string (source path) or two strings "
"(source and destination paths).",
)

arg_info = ArgumentInfo()
Expand Down
72 changes: 62 additions & 10 deletions build/ten_runtime/feature/test.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,19 @@ template("ten_package_test_prepare_app") {
replace_path_index = 0

foreach(replace_path, invoker.replace_paths_after_install_app) {
replace_src_path = replace_path
replace_dest_path = "${test_case_out_dir_rel_path}/${replace_path}"
replace_info =
exec_script("//.gnfiles/build/scripts/get_src_and_dest_file.py",
[
"--input-string",
replace_path,
"--src-dest-delimiter",
"=>",
],
"json")

replace_src_path = replace_info.source
replace_dest_path =
"${test_case_out_dir_rel_path}/${replace_info.destination}"

action(
"${test_case_unique_target_name}_${_target_name}_after_install_app_replace_file_${replace_path_index}") {
Expand Down Expand Up @@ -216,8 +227,19 @@ template("ten_package_test_prepare_app") {
replace_path_index = 0

foreach(replace_path, invoker.replace_paths_after_install_all) {
replace_src_path = replace_path
replace_dest_path = "${test_case_out_dir_rel_path}/${replace_path}"
replace_info =
exec_script("//.gnfiles/build/scripts/get_src_and_dest_file.py",
[
"--input-string",
replace_path,
"--src-dest-delimiter",
"=>",
],
"json")

replace_src_path = replace_info.source
replace_dest_path =
"${test_case_out_dir_rel_path}/${replace_info.destination}"

action(
"${test_case_unique_target_name}_${_target_name}_after_install_all_replace_file_${replace_path_index}") {
Expand Down Expand Up @@ -350,26 +372,56 @@ template("ten_package_test_prepare_app") {
if (defined(invoker.replace_paths_after_install_app) &&
invoker.replace_paths_after_install_app != []) {
foreach(replace_path, invoker.replace_paths_after_install_app) {
replace_info = []
replace_info =
exec_script("//.gnfiles/build/scripts/get_src_and_dest_file.py",
[
"--input-string",
replace_path,
"--src-dest-delimiter",
"=>",
],
"json")

replace_src_path = replace_info.source
replace_dest_path = replace_info.destination

args += [
"--replace-paths-after-install-app",
replace_path,
replace_src_path,
replace_dest_path,
]

sources += [ replace_path ]
outputs += [ "${test_case_out_dir_rel_path}/.assemble_info/${invoker.generated_app_src_root_dir_name}/files_to_be_replaced_after_install_app/${replace_path}" ]
sources += [ replace_src_path ]
outputs += [ "${test_case_out_dir_rel_path}/.assemble_info/${invoker.generated_app_src_root_dir_name}/files_to_be_replaced_after_install_app/${replace_dest_path}" ]
}
}

if (defined(invoker.replace_paths_after_install_all) &&
invoker.replace_paths_after_install_all != []) {
foreach(replace_path, invoker.replace_paths_after_install_all) {
replace_info = []
replace_info =
exec_script("//.gnfiles/build/scripts/get_src_and_dest_file.py",
[
"--input-string",
replace_path,
"--src-dest-delimiter",
"=>",
],
"json")

replace_src_path = replace_info.source
replace_dest_path = replace_info.destination

args += [
"--replace-paths-after-install-all",
replace_path,
replace_src_path,
replace_dest_path,
]

sources += [ replace_path ]
outputs += [ "${test_case_out_dir_rel_path}/.assemble_info/${invoker.generated_app_src_root_dir_name}/files_to_be_replaced_after_install_all/${replace_path}" ]
sources += [ replace_src_path ]
outputs += [ "${test_case_out_dir_rel_path}/.assemble_info/${invoker.generated_app_src_root_dir_name}/files_to_be_replaced_after_install_all/${replace_dest_path}" ]
}
}

Expand Down
5 changes: 5 additions & 0 deletions build/ten_runtime/options.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ declare_args() {
ten_enable_integration_tests = true
}

declare_args() {
# If set, enable memory leak checking in TEN go app.
ten_enable_go_app_leak_check = is_linux && is_debug && target_cpu == "x64"
}

ten_runtime_common_defines = common_defines
if (ten_enable_memory_check) {
ten_runtime_common_defines += [ "TEN_ENABLE_MEMORY_CHECK" ]
Expand Down
8 changes: 0 additions & 8 deletions core/include_internal/ten_runtime/binding/go/ten_env/ten_env_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,3 @@ void tenGoSetPropertyCallback(ten_go_handle_t ten_env, ten_go_handle_t handler,

void tenGoGetPropertyCallback(ten_go_handle_t ten_env, ten_go_handle_t handler,
ten_go_handle_t value);

void tenGoOnAddonCreateExtensionDone(ten_go_handle_t ten_env,
ten_go_handle_t addon,
ten_go_handle_t extension,
ten_go_handle_t handler);

void tenGoOnAddonDestroyExtensionDone(ten_go_handle_t ten_env,
ten_go_handle_t handler);
2 changes: 1 addition & 1 deletion core/src/ten_runtime/addon/addon.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ ten_addon_t *ten_addon_unregister(ten_addon_store_t *store,
const char *addon_name) {
TEN_ASSERT(store && addon_name, "Should not happen.");

TEN_LOGV("Unregistered addon '%s'", addon_name);
TEN_LOGI("Unregistered addon '%s'", addon_name);

return ten_addon_store_del(store, addon_name);
}
Expand Down
15 changes: 15 additions & 0 deletions core/src/ten_runtime/binding/go/interface/ten/addon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,18 @@ func tenGoAddonDestroyInstance(
)
}
}

//export tenGoAddonOnDestroy
func tenGoAddonOnDestroy(
addonID C.uintptr_t,
) {
obj := loadAndDeleteImmutableHandle(goHandle(addonID))
if obj == nil {
panic(
fmt.Sprintf(
"Failed to find addon from handle map, id: %d.",
uintptr(addonID),
),
)
}
}
23 changes: 10 additions & 13 deletions core/src/ten_runtime/binding/go/interface/ten/addon_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import "C"

import (
"fmt"
"runtime"
"sync"
"unsafe"
)

// AddonManager is a manager for addons.
type AddonManager struct {
// Define a registry map to store addon registration functions.
// The key is the addonName (string), and the value is a function that takes
Expand Down Expand Up @@ -52,7 +54,7 @@ func (am *AddonManager) RegisterAddonAsExtension(
addonID := newImmutableHandle(addonWrapper)

var bridge C.uintptr_t
cgo_error := C.ten_go_addon_register_extension(
cgoError := C.ten_go_addon_register_extension(
unsafe.Pointer(unsafe.StringData(addonName)),
C.int(len(addonName)),
cHandle(addonID),
Expand All @@ -66,13 +68,17 @@ func (am *AddonManager) RegisterAddonAsExtension(
&bridge,
)

if err := withCGoError(&cgo_error); err != nil {
if err := withCGoError(&cgoError); err != nil {
loadAndDeleteImmutableHandle(addonID)
return err
}

addonWrapper.cPtr = bridge

runtime.SetFinalizer(addonWrapper, func(p *addon) {
C.ten_go_addon_unregister(p.cPtr)
})

return nil
}

Expand Down Expand Up @@ -109,23 +115,14 @@ func (am *AddonManager) RegisterAllAddons(registerCtx interface{}) error {
return nil
}

// unloadAllAddons unloads all addons.
func (am *AddonManager) unloadAllAddons() error {
clearImmutableHandles(func(value any) {
if addon, ok := value.(*addon); ok {
C.ten_go_addon_unregister(addon.cPtr)
}
})

return nil
}

var defaultAddonManager = newAddonManager()

// RegisterAddonAsExtension registers the addon as an extension.
func RegisterAddonAsExtension(addonName string, instance Addon) error {
return defaultAddonManager.RegisterAddonAsExtension(addonName, instance)
}

// RegisterAllAddons executes all registered addon registration functions.
func RegisterAllAddons(registerCtx interface{}) error {
return defaultAddonManager.RegisterAllAddons(registerCtx)
}
2 changes: 1 addition & 1 deletion core/src/ten_runtime/binding/go/interface/ten/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func tenGoAppOnConfigure(
panic("Invalid ten object type.")
}

tenEnvInstance.attachToApp(appObj)
tenEnvInstance.attachToApp()

appObj.OnConfigure(tenEnvObj)
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/ten_runtime/binding/go/interface/ten/audio_frame.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

package ten

//#include <stdbool.h>
//#include "audio_frame.h"
// #include <stdbool.h>
// #include "audio_frame.h"
import "C"

import "unsafe"
Expand Down
Loading

0 comments on commit a537e7f

Please sign in to comment.