Memory leak when calling guest functions with env-vars

[wujunzhuo]

1. This is a very simple Rust guest function, which just prints out an environment variable USER:

   #[no_mangle]
   pub extern "C" fn hello() {
       println!("hello: {:?}", std::env::var("USER").ok());
   }

2. If we execute this function repeatedly, then we will see the program can load env-vars correctly; but the program memory usage keeps increasing. (I've also tested other wasm implementations as well, e.g. WasmEdge crate)

   use anyhow::Result;
   use wasmtime::{Engine, Linker, Module, Store};
   use wasmtime_wasi::WasiCtxBuilder;
   
   const WASM_FILE: &str = "/tmp/hello.wasm";
   
   fn main() -> Result<()> {
       let engine = Engine::default();
       let mut linker = Linker::new(&engine);
       wasmtime_wasi::add_to_linker(&mut linker, |s| s)?;
       let wasi = WasiCtxBuilder::new().inherit_env()?.inherit_stdio().build();
       let mut store = Store::new(&engine, wasi);
       let module = Module::from_file(&engine, WASM_FILE)?;
       let instance = linker.instantiate(&mut store, &module)?;
       let func = instance.get_typed_func::<(), ()>(&mut store, "hello")?;
       loop {
           func.call(&mut store, ())?;
       }
   }

   wujunzhuo@wujunzhuo-macbook demo % ./target/debug/hello
   hello: Some("wujunzhuo")
   hello: Some("wujunzhuo")
   hello: Some("wujunzhuo")
   hello: Some("wujunzhuo")

   # monitor memory
   wujunzhuo@wujunzhuo-macbook Desktop % do
       sleep 1
       ps -p $(pidof hello) -o %mem,rss
   done
   
   %MEM    RSS
   0.4  65536
   %MEM    RSS
   0.5  80032
   %MEM    RSS
   0.6  94496
   %MEM    RSS
   0.6 108928
   %MEM    RSS
   0.7 123376
   %MEM    RSS
   0.8 137824
   %MEM    RSS
   0.9 152288
   %MEM    RSS
   1.0 166688
   %MEM    RSS
   1.1 181168

3. I found the __wasilibc_initialize_environ function might be the root cause of the memory leak, and after changing some lines like the following, the program could run normally.

   wujunzhuo@wujunzhuo-macbook wasi-libc % git diff
   diff --git a/libc-bottom-half/sources/__wasilibc_initialize_environ.c b/libc-bottom-half/sources/__wasilibc_initialize_environ.c
   index 2d31c5d..65738d1 100644
   --- a/libc-bottom-half/sources/__wasilibc_initialize_environ.c
   +++ b/libc-bottom-half/sources/__wasilibc_initialize_environ.c
   @@ -15,7 +15,7 @@ weak char **__wasilibc_environ = (char **)-1;
   
   // See the comments in libc-environ.h.
   void __wasilibc_ensure_environ(void) {
   -    if (__wasilibc_environ == (char **)-1) {
   +    if (__wasilibc_environ == (char **)-1 || __wasilibc_environ == NULL) {
           __wasilibc_initialize_environ();
       }
   }
   diff --git a/libc-bottom-half/sources/environ.c b/libc-bottom-half/sources/environ.c
   index 50d60de..fedfc83 100644
   --- a/libc-bottom-half/sources/environ.c
   +++ b/libc-bottom-half/sources/environ.c
   @@ -20,12 +20,12 @@ weak_alias(__wasilibc_environ, environ);
   // reserved things to go before or after.
   __attribute__((constructor(50)))
   static void __wasilibc_initialize_environ_eagerly(void) {
   -    __wasilibc_initialize_environ();
   +    __wasilibc_ensure_environ();
   }
   
   // See the comments in libc-environ.h.
   void __wasilibc_maybe_reinitialize_environ_eagerly(void) {
       // This translation unit is linked in if `environ` is used, meaning we need
       // to eagerly reinitialize the environment variables.
   -    __wasilibc_initialize_environ();
   +    __wasilibc_ensure_environ();
   }

   %MEM    RSS
   0.2  25680
   %MEM    RSS
   0.2  28336
   %MEM    RSS
   0.2  28336
   %MEM    RSS
   0.2  28336
   %MEM    RSS
   0.2  28336
   %MEM    RSS
   0.2  28336
   %MEM    RSS
   0.2  28336
   %MEM    RSS
   0.2  28336

   However, I can't figure out why __wasilibc_environ might be initialized to NULL: since if we only compare it with -1, the wasm function couldn't load any environment variable:

   wujunzhuo@wujunzhuo-macbook demo % ./target/debug/hello
   hello: None
   hello: None
   hello: None

[wujunzhuo]

After creating this issue I've carefully read the code comments and reviewed the commit history logs related to environ, then I opened a pull request #390 . Hope this pr could help to solve the memory leak problem. @sunfishcode

[sunfishcode]

If __wasilibc_initialize_environ_eagerly is being called repeatedly, it means __wasm_call_ctors is being called repeatedly, which is not supposed to ever happen. C++ code using static constructors or C code using __attribute__((constructor)) or equivalent in general are not supposed to have to thiink about these functions being called multiple times.

I believe we need to further build out the infrastructure for reactors/libraries so that toolchains can ensure that __wasm_call_ctors is only called once in a program with no main that accepts arbitrary export calls.

[wujunzhuo]

@sunfishcode Thanks for your reply~

it means __wasm_call_ctors is being called repeatedly, which is not supposed to ever happen.

Yes indeed. I used wasm2wat tool to translate the binary format wasm file. We can see the exported function hello is wrapped with __wasm_call_ctors and __wasm_call_dtors, hence both of them will be calling multiple times.

[wujunzhuo]

update:

After Rust v1.69.0, this problem has been fixed.