How to new an em-instance without INST.DELETE() and without memory LEAK?

[haelue]

I use embind and .smart_ptr_constructor() to bind a C++ class.

I suppose a smart-pointer could help me to write javascript easy without calling .delete(), but the browser console still warning:

xmc.js:1423 Embind found a leaked C++ instance Random <0x000113d8>.

Since .delete() is easily interrupted by exception or something else, how can I use emscripten really safe and concise?

My C++ header:

#ifndef RANDOM_H
#define RANDOM_H

#include <memory>

extern "C" {
    /**
     * @brief Generates random number.
    */
    class Random {
    private:
        long long seed;
    public:
        /**
         * @brief Construct randomizer using a seed.
         * WITH fixed-number to get false-random.
         * WITH timestamp to get real-random.
        */
        Random(double seed);
        int next();
        float nextFloat();
        double nextDouble();
    };

    std::shared_ptr<Random> newRandom(double seed);
}

#endif

My C++ soure:

#include "random.h"

Random::Random(double seed) {
    this->seed = (long long)seed;
    if (this->seed < 0) {
        this->seed = -this->seed;
    }
    if (this->seed >= 233280) {
        this->seed = this->seed % 233280;
    }
}

int Random::next() {
    this->seed = (this->seed * 9301 + 49297) % 233280;
    return this->seed;
}

float Random::nextFloat() {
    return (float) this->next() / 233280.0f;
}

double Random::nextDouble() {
    return (double) this->next() / 233280.0;
}

std::shared_ptr<Random> newRandom(double seed) {
    return std::make_shared<Random>(seed);
}

My embind wrapper:

#include "random.h"
#include <emscripten/bind.h>

using namespace emscripten;

EMSCRIPTEN_BINDINGS(module){
    class_<Random>("Random")
    .smart_ptr_constructor("Random", &std::make_shared<Random, double>)
    .function("next", &Random::next)
    .function("nextFloat", &Random::nextFloat)
    .function("nextDouble", &Random::nextDouble);
    function("newRandom", &newRandom);
}

My build command:

em++ --bind -O0 -s SINGLE_FILE=1 -s EXPORT_NAME=xmc -I include src/random.cpp js/em_bind/xmcwrap.cpp -o js/lib/xmc.js

My javascript test:

<script type="text/javascript" src="../lib/xmc.js"></script>
<script type="module">
      async function waitWasm() {
        return new Promise((resolve) => {
          if (Module["calledRun"]) {
            return resolve();
          }
          window.Module.onRuntimeInitialized = function() {
            return resolve();
          }
        });
      }

      await waitWasm();

      const rand = new Module.Random(new Date().getTime());

      const r3 = [];
      for (let i = 0; i < 10; i++) {
        r3.push(rand.next())
      }

      const r4 = [];
      for (let i = 0; i < 10; i++) {
        r4.push(rand.nextFloat())
      }

      const r5 = [];
      for (let i = 0; i < 10; i++) {
        r5.push(rand.nextDouble())
      }
      
      console.log(r3, r4, r5);
</script>

[brendandahl]

Smart pointers will automatically be cleaned up when garbage collected, but it's still recommended you called delete on them. See https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#memory-management

[haelue]

Smart pointers will automatically be cleaned up when garbage collected, but it's still recommended you called delete on them. See https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#memory-management

So the warning "Embind found a leaked C++ instance Random <0x000113d8>" in browser console (chrome), means the garbage collect is invoking smart pointer clean-up?

[RReverser]

So the warning "Embind found a leaked C++ instance Random <0x000113d8>" in browser console (chrome), means the garbage collect is invoking smart pointer clean-up?

Yes, but that doesn't work reliably and might not work at all in some browsers. That's why manual .delete() is necessary for now.

This is similar to other usecases that require manual cleanup in JS - e.g. closing files in Node.js or other examples listed in https://github.com/tc39/proposal-explicit-resource-management. In the future we should be able to leverage that proposal to at least provide a nicer syntax for this, as in, you will be able to write

using embindObj = new EmbindClass();

and it will be freed upon exit from scope, but for now manual .delete() is the way to go.

[brendandahl]

Yes, but that doesn't work reliably and might not work at all in some browsers. That's why manual .delete() is necessary for now.

Is that still the case for recent browsers or are there still issues?

[RReverser]

Is that still the case for recent browsers or are there still issues?

It is because that check uses FinalizationRegistry which, per spec, is not guaranteed to run either anytime soon or ever (and, indeed, will not run under some GC flags). Which makes it useful for eventual cleanup and leak detection if user forgot to do so manually, but not a full replacement for a deterministic cleanup.

It can be especially problematic if C++ class owns external resources (files / databases / etc) or if C++ objects need to be dropped in predetermined order (which is somewhat common in native code where one object holds references to another and wrong destructor order can lead to memory corruption).

[kripken]

It is because that check uses FinalizationRegistry which, per spec, is not guaranteed to run either anytime soon or ever (and, indeed, will not run under some GC flags).

By flags, do you mean a flag to not collect at all? I agree we won't get callbacks in that case, but then I think it's the expected behavior.

It can be especially problematic if C++ class owns external resources (files / databases / etc) or if C++ objects need to be dropped in predetermined order

This sounds like a serious problem that I hadn't considered. It seems like this means C++ and Rust and all other languages using some form of deterministic ordered destruction can't work without manual destruction in JS..?

[YuriTu]

same issue here

[RReverser]

By flags, do you mean a flag to not collect at all?

Either not collect at all, or simply high GC limits (which are configurable in JS engines) which will also cause GC to never be hit. In that scenario it will also vary on the device, available memory and so on, becoming pretty hard to debug.

It seems like this means C++ and Rust and all other languages using some form of deterministic ordered destruction can't work without manual destruction in JS..?

Yes, deterministic destruction is exactly why tc39/proposal-explicit-resource-management is being proposed even though FinalizationRegistry already exists. I believe it will cover those cases better.

For some (most?) usecases users can still use FinalizationRegistry on per-class basis if they're sure it's fine for the given class to run destructor out of order or not run it at all, I'm just saying it shouldn't be the default but rather an explicit opt-in. Maybe as a new Embind annotation?