Skip to content

Commit 3fa5c8c

Browse files
Alzatharwsfulton
authored andcommitted
Fix R memory leak on exception
There is a possible memory leak in case the SWIG_exception_fail macro is called. The problem is related to its definition that call the function Rf_warning. This function (as well as Rf_error) involves a longjmp over C++ destructors on the stack. Thus, all the objects allocated on the heap are not freed. Closes swig#914
1 parent b0ce226 commit 3fa5c8c

File tree

6 files changed

+129
-8
lines changed

6 files changed

+129
-8
lines changed

Examples/test-suite/r/Makefile.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ C_TEST_CASES += \
1818

1919
CPP_TEST_CASES += \
2020
r_double_delete \
21+
r_memory_leak \
2122
r_overload_array \
2223
r_sexp \
2324
r_overload_comma \
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
clargs <- commandArgs(trailing=TRUE)
2+
source(file.path(clargs[1], "unittest.R"))
3+
4+
dyn.load(paste("r_memory_leak", .Platform$dynlib.ext, sep=""))
5+
source("r_memory_leak.R")
6+
cacheMetaData(1)
7+
8+
a <- Foo();
9+
unittest(Foo_get_count(), 1);
10+
b <- Foo();
11+
unittest(Foo_get_count(), 2);
12+
13+
# Normal behaviour
14+
invisible(trigger_internal_swig_exception("no problem", a));
15+
unittest(Foo_get_count(), 2);
16+
# SWIG exception introduced
17+
result <- tryCatch({
18+
trigger_internal_swig_exception("null", b);
19+
}, warning = function(w) {
20+
# print(" Hum... We received a warning, but this should be an error");
21+
unittest(1,0);
22+
}, error = function(e) {
23+
# print(" Gotcha!");
24+
unittest(1,1);
25+
})
26+
unittest(Foo_get_count(), 2);

Examples/test-suite/r_memory_leak.i

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
%module r_memory_leak
2+
3+
%include <std_string.i>
4+
5+
%typemap(in) Foo* foo
6+
{
7+
$1 = new Foo;
8+
}
9+
%typemap(freearg) Foo* foo
10+
{
11+
printf(" \" Object deleted\"\n");
12+
delete $1;
13+
}
14+
%typemap(out) Foo* verify_no_memory_leak
15+
{
16+
if ($1 == NULL)
17+
SWIG_exception_fail(SWIG_RuntimeError, "Let's see how the bindings manage this exception!");
18+
}
19+
%typemap(scoerceout) Foo*
20+
%{ if (!is.null($result) && !is.logical($result)) {$result <- new("$R_class", ref=$result) ;} %}
21+
22+
%inline %{
23+
#include <string>
24+
25+
class Foo {
26+
static unsigned count;
27+
public:
28+
Foo() { ++count; }
29+
~Foo() { --count; }
30+
static unsigned get_count() { return count; }
31+
};
32+
33+
unsigned Foo::count = 0;
34+
35+
static Foo* trigger_internal_swig_exception(const std::string& message, Foo* foo)
36+
{
37+
return (message == "null") ? NULL : foo;
38+
};
39+
40+
%}

Lib/r/rfragments.swg

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
#define SWIG_Error(code, msg) Rf_warning(msg); return Rf_ScalarLogical(NA_LOGICAL)
2-
3-
#define SWIG_fail return Rf_ScalarLogical(NA_LOGICAL)
4-
51
/* for raw pointers */
62
#define SWIG_ConvertPtr(oc, ptr, ty, flags) SWIG_R_ConvertPtr(oc, ptr, ty, flags)
73
#define SWIG_ConvertFunctionPtr(oc, ptr, ty) SWIG_R_ConvertPtr(oc, ptr, ty, 0)

Lib/r/rrun.swg

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,46 @@
1+
#include <stdarg.h> // va_list, va_start, va_end
2+
#include <stdio.h> // vsnprintf
3+
4+
/* Last error */
5+
static int SWIG_lasterror_code = 0;
6+
static char SWIG_lasterror_msg[1024];
7+
SWIGRUNTIME void SWIG_Error(int code, const char *format, ...) {
8+
va_list arg;
9+
SWIG_lasterror_code = code;
10+
va_start(arg, format);
11+
vsnprintf(SWIG_lasterror_msg, sizeof(SWIG_lasterror_msg), format, arg);
12+
va_end(arg);
13+
}
14+
15+
SWIGRUNTIME const char* SWIG_ErrorType(int code) {
16+
switch (code) {
17+
case SWIG_MemoryError:
18+
return "SWIG:MemoryError";
19+
case SWIG_IOError:
20+
return "SWIG:IOError";
21+
case SWIG_RuntimeError:
22+
return "SWIG:RuntimeError";
23+
case SWIG_IndexError:
24+
return "SWIG:IndexError";
25+
case SWIG_TypeError:
26+
return "SWIG:TypeError";
27+
case SWIG_DivisionByZero:
28+
return "SWIG:DivisionByZero";
29+
case SWIG_OverflowError:
30+
return "SWIG:OverflowError";
31+
case SWIG_SyntaxError:
32+
return "SWIG:SyntaxError";
33+
case SWIG_ValueError:
34+
return "SWIG:ValueError";
35+
case SWIG_SystemError:
36+
return "SWIG:SystemError";
37+
case SWIG_AttributeError:
38+
return "SWIG:AttributeError";
39+
}
40+
return "SWIG:UnknownError";
41+
}
42+
43+
#define SWIG_fail goto fail
144

245
/* Remove global namespace pollution */
346
#if !defined(SWIG_NO_R_NO_REMAP)

Source/Modules/r.cxx

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1985,7 +1985,9 @@ int R::functionWrapper(Node *n) {
19851985
for (p = l; p;) {
19861986
if ((tm = Getattr(p, "tmap:freearg"))) {
19871987
Replaceall(tm, "$source", Getattr(p, "lname"));
1988-
Printv(cleanup, tm, "\n", NIL);
1988+
if (tm && (Len(tm) != 0)) {
1989+
Printv(cleanup, tm, "\n", NIL);
1990+
}
19891991
p = Getattr(p, "tmap:freearg:next");
19901992
} else {
19911993
p = nextSibling(p);
@@ -2066,8 +2068,10 @@ int R::functionWrapper(Node *n) {
20662068
}
20672069

20682070
/* Output cleanup code */
2069-
Printv(f->code, cleanup, NIL);
2070-
Delete(cleanup);
2071+
int need_cleanup = Len(cleanup) != 0;
2072+
if (need_cleanup) {
2073+
Printv(f->code, cleanup, NIL);
2074+
}
20712075

20722076
/* Look to see if there is any newfree cleanup code */
20732077
if (GetFlag(n, "feature:new")) {
@@ -2124,7 +2128,18 @@ int R::functionWrapper(Node *n) {
21242128
if (destructor)
21252129
Printv(f->code, "R_ClearExternalPtr(self);\n", NIL);
21262130

2127-
Printv(f->code, "return r_ans;\n}\n", NIL);
2131+
Printv(f->code, "return r_ans;\n", NIL);
2132+
2133+
/* Error handling code */
2134+
Printv(f->code, "fail: SWIGUNUSED;\n", NIL);
2135+
if (need_cleanup) {
2136+
Printv(f->code, cleanup, NIL);
2137+
}
2138+
Printv(f->code, " Rf_error(\"%s %s\", SWIG_ErrorType(SWIG_lasterror_code), SWIG_lasterror_msg);\n", NIL);
2139+
Printv(f->code, " return R_NilValue;\n", NIL);
2140+
Delete(cleanup);
2141+
2142+
Printv(f->code, "}\n", NIL);
21282143
Printv(sfun->code, "\n}", NIL);
21292144

21302145
/* Substitute the function name */

0 commit comments

Comments
 (0)