Skip to content

Commit f439f12

Browse files
committed
Do major refactoring re: resource management.
Changes to the API: * Most types have a Close() method, which can be used to explicitly free the memory used by the type. The garbage collector will still pick them up eventually, but since the go GC sometimes doesn't make the smartest decisions about foreign objects, this is available. Functional changes to the implementation: * Close() methods (which are called by GC) are now much safer and more robust: * They are now thread-safe. * It is safe to call them more than once. * It is safe to call Close() on a child object whose parent has already been Close()d. The above issues were causing runtime panics under various circumstances. There are a few of these still to hunt down. The details of the internals are described in the file README-INTERNALS.md Organizational changes to the implementation: * Everything with a Close() method is defined in terms of a type cStruct, which captures most of the common functionality between them. There's generally less boilerplate, and allows only writing the tricky synchronization stuff for use with Close()/GC once.
1 parent 282418a commit f439f12

10 files changed

+374
-188
lines changed

README-INTERNALS.md

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
This document describes some general patterns in the implementation.
2+
3+
# Resource reclamation.
4+
5+
The library is written such that all resources will be released when the
6+
garbage collector claims an object. However, we also export Close()
7+
methods on each type that needs explicit cleanup. This is because the Go
8+
garbage collector [isn't always able to make the right decisions about
9+
objects with pointers to C memory][1].
10+
11+
## Tracking dependencies
12+
13+
Each notmuch object has a corresponding wrapper object:
14+
notmuch_database_t is wrapped by DB, notmuch_query_t is wrapped by Query
15+
and so on. Each of these wrappers is an alias for the type cStruct,
16+
which holds a pointer to the underlying C object, and also to the
17+
wrappers for any objects referenced by the underlying C object (via the
18+
"parents" field). This keeps the GC from collecting parent objects if
19+
the children are still in use.
20+
21+
## Creating objects
22+
23+
When creating an object, the caller should set the parents field to a
24+
slice containing pointers to the object's immediate parent objects, and
25+
set cptr to the underlying c pointer. Finally, calling setGcClose on the
26+
object will cause it to be released properly by the garbage collector.
27+
28+
## Cleaning up
29+
30+
Calling the `Close()` method on an object a second time is a no-op, and
31+
thus always safe. The primary reason for this is that it makes dealing
32+
with mixing GC and manual reclamation simple.
33+
34+
The Close methods also clear all of their references to parent objects
35+
explicitly. While this isn't strictly necessary, it means the GC
36+
will know that they are unreachable sooner, if that becomes the case.
37+
Per the documentation for `runtime.SetFinalizer`, once the finalizer is
38+
called, the object will stick around until the next time the GC looks at
39+
it. Because of this, it won't otherwise even consider parent objects
40+
until the third pass at least.
41+
42+
In some cases, invoking the `*_destroy` function on an object also
43+
cleans up its children, in which case it becomes unsafe to then invoke
44+
their `*_destroy` function. cStruct handles all of the bookkeeping and
45+
synchronization necessary to deal with this; derived types just need to
46+
make proper use of doClose() in their Close() implementation (see the
47+
comments in cstruct.go)
48+
49+
[1]: https://gist.github.com/dwbuiten/c9865c4afb38f482702e#cleaning

cstruct.go

+99
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package notmuch
2+
3+
// Copyright © 2015 The go.notmuch Authors. Authors can be found in the AUTHORS file.
4+
// Licensed under the GPLv3 or later.
5+
// See COPYING at the root of the repository for details.
6+
7+
import (
8+
"io"
9+
"runtime"
10+
"sync"
11+
"unsafe"
12+
)
13+
14+
// cStruct is the common representation of almost all of our wrapper types.
15+
//
16+
// It does the heavy lifting of interacting with the garbage
17+
// collector/*_destroy functions.
18+
type cStruct struct {
19+
// A pointer to the underlying c object
20+
cptr unsafe.Pointer
21+
22+
// Parent object. Holding a pointer to this in Go-land makes the reference
23+
// visible to the garbage collector, and thus prevents it from being
24+
// reclaimed prematurely.
25+
parent *cStruct
26+
27+
// readers-writer lock for dealing with mixing manual calls to Close() with
28+
// GC.
29+
lock sync.RWMutex
30+
}
31+
32+
// Recursively acquire read locks on this object and all parent objects.
33+
func (c *cStruct) rLock() {
34+
c.lock.RLock()
35+
if c.parent != nil {
36+
c.parent.rLock()
37+
}
38+
}
39+
40+
// Recursively release read locks this object and all parent objects.
41+
func (c *cStruct) rUnlock() {
42+
if c.parent != nil {
43+
c.parent.rUnlock()
44+
}
45+
c.lock.RUnlock()
46+
}
47+
48+
// Call f in a context in which it is safe to destroy the underlying C object.
49+
// `f` will only be invoked if the underlying object is still live. When `f`
50+
// is invoked, The calling goroutine will hold the necessary locks to make
51+
// destroying the underlying object safe.
52+
//
53+
// Typically, wrapper types will use this to implement their Close() methods;
54+
// it handles all of the synchronization bits.
55+
func (c *cStruct) doClose(f func() error) error {
56+
// Briefly:
57+
// 1. Acquire a write lock on ourselves.
58+
// 2. Acquire read locks for all of our ancestors in pre-order (the ordering
59+
// is important to avoid deadlocks).
60+
// 3. Check if we're live, and call f if so.
61+
// 4. Clear all of our references to other objects, and release the locks
62+
var err error
63+
c.lock.Lock()
64+
if c.parent != nil {
65+
c.parent.rLock()
66+
}
67+
if c.live() {
68+
err = f()
69+
}
70+
if c.parent != nil {
71+
c.parent.rUnlock()
72+
}
73+
c.cptr = nil
74+
c.parent = nil
75+
c.lock.Unlock()
76+
return err
77+
}
78+
79+
// Returns true if and only if c's underlying object is live, i.e. neither it
80+
// nor any of its ancestor objects have been finalized.
81+
//
82+
// Note that this method does no synchronization; the caller must separately
83+
// acquire the necessary locks.
84+
func (c *cStruct) live() bool {
85+
if c.cptr == nil {
86+
return false
87+
}
88+
if c.parent == nil {
89+
return true
90+
}
91+
return c.parent.live()
92+
}
93+
94+
// Set a finalizer to invoke c.Close() when c is garbage collected.
95+
func setGcClose(c io.Closer) {
96+
runtime.SetFinalizer(c, func(c io.Closer) {
97+
c.Close()
98+
})
99+
}

0 commit comments

Comments
 (0)