Should const reference values passed to JS via the "call" method be deleted in JS?

[Dachong233]

I am new to C++
I recently had some memory leaks in my project and used Address Sanitizer to locate the leak. But when I tried to fix it as described on the home page, it didn't work (I specified rvp::reference).

// wrapper class
class JsLayoutDrawerListenerWrapper : public wrapper<LayoutDrawerListener> {
 public:
  EMSCRIPTEN_WRAPPER(JsLayoutDrawerListenerWrapper);

  uint32_t FetchThemeColor(ThemeCategory style_type,
                           const MyColor &color) override {
    auto argb = call<uint32_t >("FetchThemeColor", style_type, color);
    return argb;
  }
};

// bind
class_<LayoutDrawerListener>("LayoutDrawerListener")
      .allow_subclass<JsLayoutDrawerListenerWrapper>(
          "JsLayoutDrawerListenerWrapper")
      .function("FetchThemeColor", &LayoutDrawerListener::FetchThemeColor, return_value_policy::reference());

// leak report from asan
epub.js:1409 Direct leak of 1044 byte(s) in 87 object(s) allocated from:
epub.js:1409     #0 0x1b1be8 in malloc+0x1b1be8 (this.program+0x1b1be8)
epub.js:1409     #1 0x18e89f in operator_new_impl(unsigned long)+0x18e89f (this.program+0x18e89f)
epub.js:1409     #2 0x18e79c in operator new(unsigned long)+0x18e79c (this.program+0x18e79c)
epub.js:1409     #3 0xc3a5e in unsigned int emscripten::val::internalCall<(emscripten::internal::EM_METHOD_CALLER_KIND)0, unsigned int, unsigned int emscripten::val::call<unsigned int, ThemeCategory&, MyColor const&>(char const*, ThemeCategory&, MyColor const&) const::'lambda'(emscripten::internal::_EM_METHOD_CALLER*, emscripten::_EM_VAL*, emscripten::internal::_EM_DESTRUCTORS**, void const*), ThemeCategory&, MyColor const&>(unsigned int emscripten::val::call<unsigned int, ThemeCategory&, MyColor const&>(char const*, ThemeCategory&, MyColor const&) const::'lambda'(emscripten::internal::_EM_METHOD_CALLER*, emscripten::_EM_VAL*, emscripten::internal::_EM_DESTRUCTORS**, void const*), ThemeCategory&, MyColor const&) const+0xc3a5e (this.program+0xc3a5e)
epub.js:1409     #4 0xc3961 in JsLayoutDrawerListenerWrapper::FetchThemeColor(ThemeCategory, MyColor const&)+0xc3961 (this.program+0xc3961)
epub.js:1409     #5 0xdbc8f in LayoutDrawer::ConvertRunStyle(Style const&, TextAttachment*)+0xdbc8f (this.program+0xdbc8f)
epub.js:1409     #6 0xdb013 in LayoutDrawer::DrawTextRun(Paragraph const&, unsigned int, unsigned int, float, float, float, float, float, float, float, float)+0xdb013 (this.program+0xdb013)

If you pass in the rvp::reference when binding the method, as described in the home page, a memory leak will still occur unless I manually delete the object in JS. I am not sure whether my understanding is wrong, I am confused about this, could you please tell me what the correct way is? I really appreciate your help!

// part of the code in wire.h
template<typename T>
struct BindingType<T&> : public BindingType<T> {
};

template<typename T>
struct BindingType<const T&> : public BindingType<T> {
};

template<typename T, typename>
struct BindingType : std::conditional<
    std::is_enum<T>::value,
    EnumBindingType<T>,
    GenericBindingType<T> >::type
{};

template<typename T>
struct GenericBindingType {
    typedef typename std::remove_reference<T>::type ActualT;
    typedef ActualT* WireType;

    template<typename R>
    static WireType toWireType(R&& v, rvp::default_tag) {
        return new ActualT(v);
    }

    template<typename R>
    static WireType toWireType(R&& v, rvp::take_ownership) {
        return new ActualT(std::move(v));
    }

    template<typename R>
    static WireType toWireType(R&& v, rvp::reference) {
        return &v;
    }

    static ActualT& fromWireType(WireType p) {
        return *p;
    }
};

[brendandahl]

The return value policy is for the return argument when the return type is a class. In this case it's a primitive value of uint32_t so it doesn't need a policy.

The arguments you pass into call are likely being copied, so you need to delete them on the JS side. You can confirm this if you want by adding a copy constructor that prints something for ThemeCategory and MyColor. At some point I'd like to add a "call policy" as well, so the delete isn't needed, but I haven't done any work on that.

[Dachong233]

@brendandahl Thank you very much for your help. I think I misunderstood the meaning of return_value_policy. It sounds like "call policy" would be a great idea, not only to improve some performance, but also to reduce memory leaks caused by forgetting to delete objects on the JS side.

[arximboldi]

@Dachong233 I found a sensible workaround:
#23271 (comment)