Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in class function #21709

Open
tinyjin opened this issue Apr 6, 2024 · 2 comments
Open

Memory leak in class function #21709

tinyjin opened this issue Apr 6, 2024 · 2 comments

Comments

@tinyjin
Copy link

tinyjin commented Apr 6, 2024

I've been encountering into memory leak when I use class function binding.
Despite I explicitly called delete() function, the memory leak is still happening at doLeackCheck().

As far as my looking, the situation happens from bind.h:

// TODO: This could do a reinterpret-cast if sizeof(T) === sizeof(void*)
template<typename T>
inline T* getContext(const T& t) {
    // not a leak because this is called once per binding
    auto* ret = new T(t);
#if __has_feature(leak_sanitizer) || __has_feature(address_sanitizer)
    __lsan_ignore_object(ret);
#endif
    return ret;
}

I'm wondering this is real leak because I see the comment not a leak because this is called once per binding. However the leak sanitizer still addresses this as memory leak, and seems not to be freed even after delete() called.

I attach addr2line output in where sanitizer points:

wasm-tools addr2line -v test-wasm.wasm 0x150e8

0x150e8: void (Test::**)() emscripten::internal::getContext<void (Test::*)()>(void (Test::* const&)()) /Users/jinny/Dev/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/bind.h:718:17
	void emscripten::internal::RegisterClassMethod<void (Test::*)()>::invoke<Test, >(char const*, void (Test::*)()) /Users/jinny/Dev/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/bind.h:1448:13
	emscripten::class_<Test, emscripten::internal::NoBaseClass> const& emscripten::class_<Test, emscripten::internal::NoBaseClass>::function<emscripten::internal::DeduceArgumentsTag, void (Test::*)(), >(char const*, void (Test::*)(), ) const /Users/jinny/Dev/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/bind.h:1725:9
	embind_init_test_bindings() /Users/jinny/Work/jinny/emc-test/build_wasm/../test.cpp:45:10

Appreciated if I can ensure things:

  1. Should I ignore memory leak in getContext()?
  2. If so, Isn't it really leaking memory? and why not ignored by __lsan_ignore_object?
  3. In case this is real leak, hoping it's going to be resolved.

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.56-git
clang version 19.0.0git
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /opt/homebrew/Cellar/emscripten/3.1.56/libexec/llvm/bin

Full link command and output with -v appended:
I've tried in meson/ninja build system. It would be better if you can refer to the repro repo here:
https://github.com/tinyjin/wasm-emscripten-leak

The Meson build system
Version: 1.3.2
Source dir: /Users/jinny/Work/jinny/emc-test
Build dir: /Users/jinny/Work/jinny/emc-test/build_wasm
Build type: cross build
Project name: test
Project version: 0.0.1
C++ compiler for the host machine: /Users/jinny/Dev/emsdk/upstream/emscripten/em++.py (emscripten 3.1.56 "emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.56 (cf90417346b78455089e64eb909d71d091ecc055)")
C++ linker for the host machine: /Users/jinny/Dev/emsdk/upstream/emscripten/em++.py ld.wasm 19.0.0
C++ compiler for the build machine: c++ (clang 15.0.0 "Apple clang version 15.0.0 (clang-1500.3.9.4)")
C++ linker for the build machine: c++ ld64 1053.12
Build machine cpu family: aarch64
Build machine cpu: aarch64
Host machine cpu family: x86
Host machine cpu: i686
Target machine cpu family: x86
Target machine cpu: i686
Build targets in project: 1

test 0.0.1

  User defined options
    Cross files    : /tmp/.cross.txt
    default_library: static

Found ninja-1.11.1 at /opt/homebrew/bin/ninja
WARNING: Running the setup command as `meson [options]` instead of `meson setup [options]` is ambiguous and deprecated.
ninja: Entering directory `build_wasm/'
[2/2] Linking target test-wasm.js
em++: warning: -sERROR_ON_UNDEFINED_SYMBOLS specified multiple times. Ignoring previous value (`1`) [-Wunused-command-line-argument]
em++: warning: running limited binaryen optimizations because DWARF info requested (or indirectly required) [-Wlimited-postlink-optimizations]
-rwxr-xr-x  1 jinny  staff  1779064 Apr  6 12:04 build_wasm/test-wasm.wasm
-rw-r--r--  1 jinny  staff   206880 Apr  6 12:04 build_wasm/test-wasm.js

Then If I try run this module in JS, 8 bytes direct leak occurred in each class function binding:

Direct leak of 8 byte(s) in 1 object(s) allocated from:
put_char @ test-wasm.js:1408
test-wasm.js:1408     #0 0x5075 in operator new(unsigned long)+0x5075 (http://0.0.0.0:4000/build_wasm/test-wasm.wasm+0x5075)
put_char @ test-wasm.js:1408
test-wasm.js:1408     #1 0x15144 in embind_init_test_bindings()+0x15144 (http://0.0.0.0:4000/build_wasm/test-wasm.wasm+0x15144)
put_char @ test-wasm.js:1408
test-wasm.js:1408     #2 0x15053 in _GLOBAL__sub_I_test.cpp+0x15053 (http://0.0.0.0:4000/build_wasm/test-wasm.wasm+0x15053)
put_char @ test-wasm.js:1408
test-wasm.js:1408     #3 0x9d8 in __wasm_call_ctors+0x9d8 (http://0.0.0.0:4000/build_wasm/test-wasm.wasm+0x9d8)
put_char @ test-wasm.js:1408
test-wasm.js:1408     #4 0x800003b0 in callRuntimeCallbacks http://0.0.0.0:4000/build_wasm/test-wasm.js:944:26
put_char @ test-wasm.js:1408
test-wasm.js:1408     #5 0x8000016b in initRuntime http://0.0.0.0:4000/build_wasm/test-wasm.js:363:3
put_char @ test-wasm.js:1408
test-wasm.js:1408     #6 0x80001729 in doRun http://0.0.0.0:4000/build_wasm/test-wasm.js:5929:5
put_char @ test-wasm.js:1408
test-wasm.js:1408 
put_char @ test-wasm.js:1408

SUMMARY: LeakSanitizer: 32 byte(s) leaked in 4 allocation(s).
@sbc100
Copy link
Collaborator

sbc100 commented Apr 8, 2024

I can't seem to reproduce this. Admittedly I simplified the test case a little by adding this main function and running it under node:

 int main() {                                                                     
  EM_ASM({                                                                       
    const test = new Module.Test();                                              
    test.delete();                                                               
  });                                                                                                                   
}
$ ./em++ test.cpp -lembind -sEXIT_RUNTIME -fsanitize=leak
$ node ./a.out.js 

I don't see any leak reports, so I guess there is something else going on with your more complex version.

One thing to try might be to remove -sEXIT_RUNTIME since you actually want your program to be long-running.

For sure it seems like that particular allocation should be ignored by lsan, as the comment says. According to the commend this is not really leak since its a one time allocation that happens at program startup and never again. I suppose there would be no benefit to trying to free it during shutdown.

@tinyjin
Copy link
Author

tinyjin commented Apr 15, 2024

@sbc100
Thank you for the quick response!

I've tried to build by directly invoking em++ command as well.

em++ test.cpp -lembind -sEXIT_RUNTIME -fsanitize=leak

Yes, this doesn't report any memory leaks, but build from meson/ninja system still raises a direct leak of 8 bytes.
I'm wondering why the leak is only reported in meson build.

Nevertheless, it's undoubtedly not an actual leak, so I consider it a minor issue.
I'll investigate how meson build triggers fake leak in sanitizer, and will share my findings with you once I uncover anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants