Asan detects leak in BSpline::derive().

[L3MON4D3]

Hi, first of all amazing work with tinyspline, it's been really helpful :D
I started compiling with asan recently, which detects a leak in BSpline::derive():

tinyspline::BSpline spline(5,1);
spline.setControlPoints({
	0,
	.3,
	.4,
	.6,
	1
});
tinyspline::BSpline df = spline.derive(); //<-- line 140

Direct leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x55f806189159 in malloc (/home/simon/Documents/Uni/Kurse/s4/CGVis/proj/build/space+0x809159)
    #1 0x55f806a4b5cd in ts_bspline_new /home/simon/Documents/Uni/Kurse/s4/CGVis/proj/libs/tinyspline/src/tinyspline.c:558:43
    #2 0x55f806a46134 in tinyspline::BSpline::BSpline() /home/simon/Documents/Uni/Kurse/s4/CGVis/proj/libs/tinyspline/src/tinysplinecxx.cxx:202:6
    #3 0x55f806a49396 in tinyspline::BSpline::derive(unsigned long, double) const /home/simon/Documents/Uni/Kurse/s4/CGVis/proj/libs/tinyspline/src/tinysplinecxx.cxx:579:22
    #4 0x55f8061ccfe1 in main /home/simon/Documents/Uni/Kurse/s4/CGVis/proj/src/main.cpp:140:34
    #5 0x7f2cdcd6cb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

The leak can be avoided with

diff --git a/src/tinysplinecxx.cxx b/src/tinysplinecxx.cxx
index a0d2c81..35b8bae 100644
--- a/src/tinysplinecxx.cxx
+++ b/src/tinysplinecxx.cxx
@@ -577,6 +577,7 @@ tinyspline::BSpline tinyspline::BSpline::toBeziers() const
 tinyspline::BSpline tinyspline::BSpline::derive(size_t n, real epsilon) const
 {
 	tinyspline::BSpline bs;
+	ts_bspline_free(&bs.spline);
 	tsStatus status;
 	if (ts_bspline_derive(&spline, n, epsilon, &bs.spline, &status))
 		throw std::runtime_error(status.message);

, but I'm not at all certain whether that is the correct fix.

[msteinbeck]

Thank you for reporting this issue. I guess the leak comes from the c code (tinyspline.c). I'll have a look.

[msteinbeck]

You are right. The problem is that BSpline's default constructor creates a three dimensional spline with one control point. By passing the wrapped tsBSpline struct to derive, the memory is leaked because derive doesn't free the memory of its output parameter. That said, this does not only affect derive but all methods returning a new BSpline instance.

[L3MON4D3]

Ahh, I see.
So either explicitly freeing each time or maybe a private constructor that doesn't allocate any memory?

[msteinbeck]

... or maybe a private constructor that doesn't allocate any memory?

I'm currently implementing this.

[msteinbeck]

I (hopefully) fixed the issue in branch memory-177. Could you please check if it now works with asan?

[L3MON4D3]

It Works!
Thank you for the quick fix!

[msteinbeck]

It looks like the assignment operators of BSpline and DeBoorNet also leak memory. I will commit a fix into branch memory-177.

[msteinbeck]

@L3MON4D3 FYI There are now memory checks (using Valgrind) in branch test-framework.