Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove runtime.SetFinalizer #905

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions cephfs/userperm.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package cephfs
import "C"

import (
"runtime"
"unsafe"

"github.com/ceph/go-ceph/internal/log"
Expand Down Expand Up @@ -49,11 +48,7 @@ func NewUserPerm(uid, gid int, gidlist []int) *UserPerm {
}
p.userPerm = C.ceph_userperm_new(
p.uid, p.gid, C.int(len(p.gidList)), cgids)
// if the go object is unreachable, we would like to free the c-memory
// since this has no other resources than memory associated with it.
// This is only valid for UserPerm objects created by new, and thus have
// the managed var set.
runtime.SetFinalizer(p, destroyUserPerm)

return p
}

Expand Down
3 changes: 0 additions & 3 deletions rados/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ func (c *Conn) Connect() error {

// Shutdown disconnects from the cluster.
func (c *Conn) Shutdown() {
if err := c.ensureConnected(); err != nil {
return
}
Comment on lines -74 to -76
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this change have to do with the rest of the patch?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// 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.

Doing the check will result in a memory leak if there is no connection based on what it says in the src documentation.

freeConn(c)
}

Expand Down
3 changes: 1 addition & 2 deletions rados/omap.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package rados
import "C"

import (
"runtime"
"unsafe"
)

Expand Down Expand Up @@ -44,7 +43,7 @@ func newGetOmapStep() *GetOmapStep {
more: (*C.uchar)(C.malloc(C.sizeof_uchar)),
rval: (*C.int)(C.malloc(C.sizeof_int)),
}
runtime.SetFinalizer(gos, opStepFinalizer)

return gos
}

Expand Down
3 changes: 0 additions & 3 deletions rados/rados.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package rados
import "C"

import (
"runtime"
"unsafe"

"github.com/ceph/go-ceph/internal/log"
Expand Down Expand Up @@ -74,7 +73,6 @@ func newConn(user *C.char) (*Conn, error) {
return nil, getError(ret)
}

runtime.SetFinalizer(conn, freeConn)
return conn, nil
}

Expand Down Expand Up @@ -107,7 +105,6 @@ func NewConnWithClusterAndUser(clusterName string, userName string) (*Conn, erro
return nil, getError(ret)
}

runtime.SetFinalizer(conn, freeConn)
return conn, nil
}

Expand Down
Loading