rados: free cluster runtime resources automatically

Release resources that are allocated while configuring the connection to
the cluster. rados_shutdown() should only be needed after a successful
call to rados_connect(), however if the connection has been configured
with non-default parameters, some of the parameters may be allocated
before connecting. rados_shutdown() will free the allocated resources,
even if there has not been a connection yet.

Note that the finalizers get executed during garbage collection, which
can be forced by calling runtime.GC() for testing.

Fixes: #109
Signed-off-by: Niels de Vos <ndevos@redhat.com>

contrib/.gitignore

@@ -0,0 +1 @@
+issue-109

contrib/Makefile

@@ -0,0 +1,13 @@
+BINARIES := issue-109
+
+build: $(BINARIES)
+clean:
+	$(RM) $(BINARIES)
+
+.PHONY: tests
+tests: issue-109
+	valgrind ./issue-109
+
+issue-109: issue-109.go
+	go build -v issue-109.go
+

contrib/issue-109.go

@@ -0,0 +1,46 @@
+// issue-109.go: analyze memory leak when rados.Conn.Connect() fails.
+//
+// build with: go build issue-109.go
+// test with: valgrind ./issue-109
+
+package main
+
+import (
+	"fmt"
+	"runtime"
+	"time"
+
+	"github.com/ceph/go-ceph/rados"
+)
+
+func main() {
+	for i := 1; i <= 100; i++ {
+		c, err := getConnection("127.0.0.1", "nobody", "secret")
+		if err != nil {
+			fmt.Printf("getConnection failed: %v\n", err)
+		} else {
+			c.Shutdown()
+		}
+	}
+
+	// force running the garbage collector
+	runtime.GC()
+	time.Sleep(time.Second)
+}
+
+func getConnection(monitors, user, key string) (*rados.Conn, error) {
+	conn, err := rados.NewConnWithUser(user)
+	if err != nil {
+		return nil, err
+	}
+	args := []string{"--client_mount_timeout", "15", "-m", monitors, "--key", key}
+	err = conn.ParseCmdLineArgs(args)
+	if err != nil {
+		return nil, fmt.Errorf("ParseCmdLineArgs: %v", err)
+	}
+	err = conn.Connect()
+	if err != nil {
+		return nil, fmt.Errorf("Connect: %v", err)
+	}
+	return conn, nil
+}

rados/conn.go

@@ -58,7 +58,7 @@ func (c *Conn) Shutdown() {
 	if err := c.ensure_connected(); err != nil {
 		return
 	}
-	C.rados_shutdown(c.cluster)
+	freeConn(c)
 }
 
 // ReadConfigFile configures the connection using a Ceph configuration file.

rados/rados.go

@@ -8,6 +8,7 @@ import "C"
 
 import (
 	"fmt"
+	"runtime"
 	"unsafe"
 )
 
@@ -45,11 +46,12 @@ func newConn(user *C.char) (*Conn, error) {
 	conn := makeConn()
 	ret := C.rados_create(&conn.cluster, user)
 
-	if ret == 0 {
+	if ret != 0 {
-		return conn, nil
-	} else {
 		return nil, RadosError(int(ret))
 	}
+
+	runtime.SetFinalizer(conn, freeConn)
+	return conn, nil
 }
 
 // NewConn creates a new connection object. It returns the connection and an
@@ -77,9 +79,27 @@ func NewConnWithClusterAndUser(clusterName string, userName string) (*Conn, erro
 
 	conn := makeConn()
 	ret := C.rados_create2(&conn.cluster, c_cluster_name, c_name, 0)
-	if ret == 0 {
+	if ret != 0 {
-		return conn, nil
-	} else {
 		return nil, RadosError(int(ret))
 	}
+
+	runtime.SetFinalizer(conn, freeConn)
+	return conn, nil
+}
+
+// freeConn releases resources that are allocated while configuring the
+// connection to the cluster. rados_shutdown() should only be needed after a
+// successful call to rados_connect(), however if the connection has been
+// configured with non-default parameters, some of the parameters may be
+// allocated before connecting. rados_shutdown() will free the allocated
+// resources, even if there has not been a connection yet.
+//
+// This function is setup as a destructor/finalizer when rados_create() is
+// called.
+func freeConn(conn *Conn) {
+	if conn.cluster != nil {
+		C.rados_shutdown(conn.cluster)
+		// prevent calling rados_shutdown() more than once
+		conn.cluster = nil
+	}
 }