rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.37k stars 687 forks source link

[c++] Consider implementing Drop for types with destructors #1840

Open Volker-Weissmann opened 4 years ago

Volker-Weissmann commented 4 years ago

Consider the following C++ code:

#include "stdio.h"
class Inner {
public:
    Inner();
    ~Inner();
};
Inner::Inner() {
    printf("constructing Inner\n");
}
Inner::~Inner() {
    printf("destructing Inner\n");
}
class Outer {
    Inner in;
public:
    Outer();
    ~Outer();
};
Outer::Outer() {
    printf("constructing Outer\n");
}
Outer::~Outer() {
    printf("destructing Outer\n");
}

If you use bindgen to execute

let mut val = Outer::new();
val.destruct();

You get the output

constructing Inner
constructing Outer
destructing Outer
destructing Inner

Which is also the output of the C++ code

#include "wrapper.cpp"
int main() {
    Outer obj;
}

If we now remove the ~Outer() deconstructor, this C++ code outputs

constructing Inner
constructing Outer
destructing Inner

But the rust code

let mut val = Outer::new();
val.destruct();

no longer compilers because Outer::destruct does not exist and the rust code

let mut val = Outer::new();

just outputs

constructing Inner
constructing Outer

which might result in memory leaks or similar bugs. A similar problem arises if you remove the default constructor.

Is it possible to automatically add constructors and destructors that call the constructors and destructors of the member variables?

kulp commented 4 years ago

From the bindgen user guide,

using those types in Rust will be nowhere near as nice as using them in C++. You will have to manually call constructors, destructors, overloaded operators, etc yourself.

I am not sure if that is the end of the story, but I think your reasonable request might be more than bindgen can reasonably provide.

emilio commented 4 years ago

Yes, you need to implement Drop yourself, because it can cause unsoundness otherwise.

For example, if bindgen would implement Drop by default, dropping Outer would end up calling ~Inner twice, once from Rust, once from C++, which is bad.

We could fix that using ManuallyDrop for members of structs that have destructors nowadays, I think.

Volker-Weissmann commented 4 years ago

Two things:

  1. You said that #1841 is the same as this issue and renamed this issue. But adding the default destructor/default constructor and calling the deconstructor using the drop trait are two seperate things. You can do the first thing without doing the second.

  2. I understand and could reproduce the problem of ~Inner being called twice. I think that some kind of hacky solution (maybe using ManuallyDrop) would be really valuable. One way to do this would be to autogen the following:

Outer* wrap_outer_construct() {
    return new Outer();
}
void wrap_outer_destruct(Outer *ptr) {
    assert(ptr != NULL);
    delete ptr;
}
struct wrap_outer {
    ptr: *mut Outer
}
impl wrap_outer {
    fn new() -> wrap_outer {
        let cptr = unsafe {
            wrap_outer_construct()
        };
        wrap_outer{ptr: cptr}
    }
}
impl Drop for wrap_outer {
    fn drop(&mut self) {
        unsafe {
            wrap_outer_destruct(self.ptr);
        }
    }
}

The disadvantage of this is that you can no longer access the member variables (or autogen getters/setters?).

emilio commented 4 years ago

You said that #1841 is the same as this issue and renamed this issue. But adding the default destructor/default constructor and calling the deconstructor using the drop trait are two seperate things. You can do the first thing without doing the second.

Well, kinda, right? If we auto-implement drop, then default constructors between Rust and C++ behave the same don't they?

I'm not sure under which circumstances a C++ compiler emits symbols for a default constructor, if it does at all. If you have info on that we can try to elaborate a solution for it.

Volker-Weissmann commented 4 years ago

You said that #1841 is the same as this issue and renamed this issue. But adding the default destructor/default constructor and calling the deconstructor using the drop trait are two seperate things. You can do the first thing without doing the second.

Well, kinda, right? If we auto-implement drop, then default constructors between Rust and C++ behave the same don't they?

I'm not sure under which circumstances a C++ compiler emits symbols for a default constructor, if it does at all. If you have info on that we can try to elaborate a solution for it.

No. #1841 is about pub fn destruct(&mut self) being called automatically, #1840 is about pub fn destruct(&mut self) existing if there is no ~MyClass() in the C++ code. The reason I opened #1840 is that adding ~MyClass(){} does not change anything in a pure C++ program, but it changes something for bindgen.

barakugav commented 1 month ago

I think its safe to automatically generate Drop for opaque types as destructors of inner fields will not be called twice. @emilio what do you think?