jaeandersson / swig

SWIG is a software development tool that connects programs written in C and C++ with a variety of high-level programming languages.
http://www.swig.org
Other
23 stars 19 forks source link

C++ destructors not being called? #36

Closed rdeits closed 9 years ago

rdeits commented 9 years ago

I just started experimenting with swig-matlab as a possible way to replace our huge amount of hand-written mexFunction code, and it looks very promising. However, I'm having trouble understanding how wrapped C++ classes are handled: it looks like the class destructor is never called, and the class may never be deallocated.

My test case is something like this:

#include <iostream>

class Foo {
public:
  Foo() {
    std::cout << "allocating a new Foo" << std::endl;
  }

  ~Foo() {
    std::cout << "destructing Foo at address " << this << std::endl;
  }
};

Using swig with python, I get the correct behavior:

>>> g = swigexample.Foo()
allocating a new Foo
>>> g = 1
destructing Foo at address: 0x2708b00

But with swig-matlab, the destructor seems to never be called, even when I clear mex or exit matlab.

>> x = swigexample.Foo()
allocating a new Foo
>> x = 1;
>> clear mex;
>> exit;

Is this an expected behavior? I see the same issue before and after f5532b84d23dff5af2d140f789e44210e7146e6a

rdeits commented 9 years ago

The problem seems to be that call to SWIG_Matlab_isOwned(argv[0]) inside the _wrap_delete_Foo function always returns false, so delete arg1 never occurs. Removing that check results in my objects being correctly deallocated.

jaeandersson commented 9 years ago

Hi! I'm back from vacation. This looks like a legit issue. Did you find a fix? If yes, feel free to make a pull request.

rdeits commented 9 years ago

I haven't yet. Overriding the call to SWIG_Matlab_isOwned to always return true trivially removes the immediate problem, but it seems likely to have unintended (dangerous) consequences later on. I'm not yet sure what the fundamental error is.

jaeandersson commented 9 years ago

OK. I have seen something similar with CasADi. I wanted to make another iteration on the design anyway, so hopefully a cleaner design will sort this out as well.

rdeits commented 9 years ago

Ok, sounds good. We'd love to be able to use this in Drake, so I'd be happy to help if I can. I don't have any experience with swig's internals, but can at the very least keep trying out whatever you have on our system.

rdeits commented 9 years ago

This seems to have been fixed in 269a40858f5eb218c67c0b15b8e6b5e1afdb4319 Thanks!