embind: Don't automatically destroy arguments passed into val calls.

[brendandahl]

Before PR #20383 val's call and operator() had different behaviors where one would automatically destroy arguments after the call and the other didn't. After that PR, they both called destroy. The automatic destruction wasn't really documented anywhere and and led to some unexpected behavior.

This changes it so neither automatically destroys the arguments which I think is more inline with the rest of embind where it's up to the user to handle object lifetimes.

Fixes: #21016 and #20095

[RReverser]

Looks like after this change deleteObject is no longer used anywhere, so can you remove its now-unused assignments in embind.js too?

[stbergmann]

Since this 6e54690 "embind: Don't automatically destroy arguments passed into val calls. (#21022)" was merged into 3.1.52, the below test program behaves differently than before, and I wonder whether that's intentional. The test setup is

$ cat test.cc
#include <iostream>
#include <memory>

#include "emscripten.h"
#include "emscripten/bind.h"

class Smart {
public:
  Smart(): id_(counter++) { std::cout << "create " << id_ << "\n"; }
  Smart(Smart const & other): id_(counter++) {
    std::cout << "copy " << other.id() << " to " << id_ << "\n";
  }
  ~Smart() { std::cout << "destroy " << id_ << "\n"; }
  int id() const { return id_; }
private:
  void operator =(Smart) = delete;
  static unsigned counter;
  int const id_;
};

unsigned Smart::counter = 0;

std::shared_ptr<Smart> getSmart() {
  return std::make_shared<Smart>();
}

struct Interface {
  virtual void memFun(Smart const &) const {};
};

struct Wrapper: emscripten::wrapper<Interface> {
public:
  EMSCRIPTEN_WRAPPER(Wrapper);
  void memFun(Smart const & smart) const override {
      call<void>("memFun", smart);
  }
};

void useInterface(Interface const & ifc) {
  std::shared_ptr<Smart> s(getSmart());
  ifc.memFun(*s);
}

EM_JS(void, jsRun, (), {
  const Impl = Module.Interface.extend('Interface', {
    memFun(smart) { console.log('use ' + smart.id()); }
  });
  const impl = new Impl();
  const s = Module.getSmart();
  impl.memFun(s);
  impl.memFun(s);
  s.delete();
  Module.useInterface(impl);
});

EMSCRIPTEN_BINDINGS(test) {
  emscripten::class_<Smart>("Smart")
    .smart_ptr<std::shared_ptr<Smart>>("SmartPtr")
    .function("id", &Smart::id);
  emscripten::class_<Interface>("Interface")
    .function("memFun", &Interface::memFun)
    .allow_subclass<Wrapper>("Wrapper");
  emscripten::function("getSmart", &getSmart);
  emscripten::function("useInterface", &useInterface);
}

int main() {
  jsRun();
}

and

$ em++ test.cc -lembind -o test.html && emrun test.html

Until 3.1.51 (also in 3.1.47 and 3.1.48, i.e., irrespective of 12777ca "[emval] Reuse method calling optimisation in regular calls (#20383)" that was merged into 3.1.48 and motivated this change), the browser console logs were

create 0
use 0
use 0
destroy 0
create 1
copy 1 to 2
use 2
destroy 2
destroy 1

That is., the instance of Smart used in the call to JS ifc.memFun(*s); from C++ useInterface is properly destroyed.

Since 3.1.52, the browser console logs instead are

create 0
use 0
use 0
destroy 0
create 1
copy 1 to 2
use 2
destroy 1

That is, the instance of Smart used in the call to JS ifc.memFun(*s); from C++ useInterface is leaked now. But even if the new behavior is intentional: Where should that instance be destroyed? I think this commit assumes that the JS implementation

    memFun(smart) { console.log('use ' + smart.id()); }

should smart.delete(); it now. But then, that would apparently break the sequence of direct JS to JS calls

  impl.memFun(s);
  impl.memFun(s);

from within jsRun.

[stbergmann]

Since this 6e54690 "embind: Don't automatically destroy arguments passed into val calls. (#21022)" was merged into 3.1.52, the below test program behaves differently than before, and I wonder whether that's intentional. [...]

I filed #22575 " Embind regression: Leaked C++ object when calling from C++ to JS" for this now, so it doesn't get lost.