6

I am accessing C++ code from Java using SWIG.

Getters in C++ usually return const references:

class B
{
public:
  const A& getA() const { return a_; }
private:
  A a_;
};

Generate SWIG wrappers. B::getA in Java returns a reference as expected. Yet JVM does not know that this reference is associated with class B. It might lead to a broken reference:

public A createA() {
    B b = new B();
    return b.getA();
}

Object returned from createA is invalid once garbage collector destroys B b and JVM knows nothing about it.

SWIG provides documentation addressing this issue: references and swig. Yet it means that I need to add all these references manually.

I came up with the following solution which returns copies instead of const references in SWIG generated code:

%typemap(out) const SWIGTYPE&
{
  *($&1_ltype)&$result = new $1_basetype(($1_type)*$1);
}
%typemap(javaout) const SWIGTYPE&
{
  return new $javaclassname($jnicall, true);
}

I have two questions:

  1. Is this approach safe? Did not I forget something?
  2. Is there any better solution to address this issue and not to write much code?

Any help is appreciated.

ifnull
  • 239
  • 2
  • 6

1 Answers1

4

That looks sensible as written however I wouldn't recommend using it.

As it stands you will lose the semantics of return by const reference if you force every case of it to become a copy. This will impact the SWIG wrapping of other things too (e.g. standard library containers).

It has the potential to adversely impact performance of your wrapped code compared to the native library. More seriously though, it fundamentally changes the behaviour in some cases too. Consider for example the following SWIG example:

%module test

%typemap(out) const SWIGTYPE&
{
  *($&1_ltype)&$result = new $1_basetype(($1_type)*$1);
}
%typemap(javaout) const SWIGTYPE&
{
  return new $javaclassname($jnicall, true);
}

%inline %{
struct A {
    int v;
    A() : v(0) {}
};

class B
{
public:
  B() { }
  const A& getA() const { return a_; }
  void counter() {
    a_.v++;
  }
  int test() const { return a_.v; }
private:
  A a_;
};

%}

If we were to use it from Java like:

public class run {
    public static void main(String[] argv) {
        System.loadLibrary("test");
        A a = test(); 
        System.out.println("A.v = " + a.getV());
    }

    private static A test() {
        B b = new B();
        A result = b.getA();
        System.out.println("A.v = " + b.test() + " (test)");
        b.counter();
        System.out.println("A.v = " + result.getV());
        return result;
    }
}

The result is that v is always (unexpectedly) 0:

A.v = 0 (test)
A.v = 0
A.v = 0

Contrast that with the same (correct) C++ beahviour:

#include <iostream>

struct A {
    int v;
    A() : v(0) {}
};

class B
{
public:
  B() { }
  const A& getA() const { return a_; }
  void counter() {
    a_.v++;
  }
  int test() const { return a_.v; }
private:
  A a_;
};

static A test();

int main() {
    const A& a = test(); 
    std::cout << "A.v = " << a.v << "\n";
}

static A test() {
    B b;
    const A& result = b.getA();
    std::cout << "A.v = " << b.test() << " (test)" << "\n";
    b.counter();
    std::cout << "A.v = " << result.v << "\n";
    return result;
}

Which correctly returns:

A.v = 0 (test)
A.v = 1
A.v = 1

That unnatural semantics is a show-stopper for me.

My suggestion would be to skip the copy constructor plan and adapt the example from the documentation to be generic:

%module test

%typemap(javabody,noblock=1) SWIGTYPE {
  private long swigCPtr;
  protected boolean swigCMemOwn;

  protected $javaclassname(long cPtr, boolean cMemoryOwn) {
    swigCMemOwn = cMemoryOwn;
    swigCPtr = cPtr;
  }

  protected $javaclassname(long cPtr, boolean cMemoryOwn, Object cParent) {
    this(cPtr, cMemoryOwn);
    swigCParent = cParent;
  }

  protected static long getCPtr($javaclassname obj) {
    return (obj == null) ? 0 : obj.swigCPtr;
  }

  private Object swigCParent;
}

%typemap(javaout) SWIGTYPE const & {
    return new $javaclassname($jnicall, $owner, this);
  }

%inline %{
struct A {
    int v;
    A() : v(0) {}
};

class B
{
public:
  B() { }
  const A& getA() const { return a_; }
  void counter() {
    a_.v++;
  }
  int test() const { return a_.v; }
private:
  A a_;
};

%}

Which now behaves like the C++ equivalent for one extra Object reference that gets inserted and managed automatically per class.

If you want to made this work automatically for both static and non-static methods we need to use a few tricks because, unfortunately, there is no way of specifying a typemap should only apply to non-static methods. The trick I used is explained in detail on another question. With this trick we can modify the SWIG interface to add:

%pragma(java) moduleimports=%{
  import java.lang.reflect.Field;
%}

%pragma(java) modulecode=%{
  static Object getThisOrNull(final Object o, final Class c) {
    for (Field f: o.getClass().getDeclaredFields()) {
      if (f.getType().equals(c)) {
        try {
          return f.get(o);
        }
        catch (IllegalAccessException e) {
          // Omm nom nom...
        }
      }
    }
    return null;
  }
%}

And change the javaout typemap to be:

%typemap(javaout) SWIGTYPE const & {
    return new $javaclassname($jnicall, $owner, $module.getThisOrNull(new Object(){}, $javaclassname.class));
  }

The remainder of the interface is as previously discussed, but this modification now causes swigCParent to be null if there is no parent (i.e. static method).

Community
  • 1
  • 1
Flexo
  • 87,323
  • 22
  • 191
  • 272
  • We have a large library exposed via swig, and early deletion (due to this issue) has been a real thorn in our side. Big thanks to @Flexo for pointing us in the right direction, and to bactron for the clear overview of the problem. Flexo's solution works for most code, but breaks on any static functions that return by reference. Any idea how to exclude static functions from a typemap? Is there a javaout modifier we can use to match statics? – Alex Goldberg Jan 29 '15 at 18:36
  • @AlexGoldberg I added a workaround that lets this compile and work for both cases in one typemap. It's ugly though, so I'd recommend running some profiling and if it hurts writing specific typemaps for specific functions. (I'm working on another solution though that might be slightly better using C++ template meta programming techniques instead) – Flexo Jan 30 '15 at 10:14
  • Wow, definitely points for creativity on that one ;) We decided that such static functions were rare enough that we could would handle them individually (by wrapping the declarations in macros that disable and then re-enable the typemap). Last thing is to figure out how to handle public member variables. Currently looking for a way to disable the automatic getter/setter creation for those... – Alex Goldberg Feb 02 '15 at 23:26
  • @AlexGoldberg You can make SWIG ignore member variables with `%rename("$ignore", %$isvariable, %$ismember) "";` - see ['Advanced Renaming'](http://www.swig.org/Doc3.0/SWIG.html#SWIG_advanced_renaming) for details. Some of the others (e.g. `%$ismemberget` or `%$isstatic`) might let you do some neat things too. – Flexo Feb 03 '15 at 08:03
  • Thanks! You've really saved us a lot of time. – Alex Goldberg Feb 03 '15 at 17:09