From 7750e85fd1ff1006a28a5e292c9bc7ce3e12b586 Mon Sep 17 00:00:00 2001 From: Carlos Martín Nieto Date: Tue, 21 Apr 2015 13:14:22 +0200 Subject: Introduce an indirection layer for pointers As the Go runtime can move stacks at any point and the C code runs concurrently with the rest of the system, we cannot assume that the payloads we give to the C code will stay valid for any particular duration. We must therefore give the C code handles which we can then look up in our own list when the callbacks get called. --- handles.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 handles.go (limited to 'handles.go') diff --git a/handles.go b/handles.go new file mode 100644 index 0000000..c0d1889 --- /dev/null +++ b/handles.go @@ -0,0 +1,82 @@ +package git + +import ( + "sync" + "unsafe" +) + +type HandleList struct { + sync.RWMutex + // stores the Go pointers + handles []interface{} + // indicates which indices are in use + set map[uintptr]bool +} + +func NewHandleList() *HandleList { + return &HandleList{ + handles: make([]interface{}, 5), + } +} + +// findUnusedSlot finds the smallest-index empty space in our +// list. You must only run this function while holding a write lock. +func (v *HandleList) findUnusedSlot() uintptr { + for i := 0; i < len(v.handles); i++ { + isUsed := v.set[uintptr(i)] + if !isUsed { + return uintptr(i) + } + } + + // reaching here means we've run out of entries so append and + // return the new index, which is equal to the old length. + slot := len(v.handles) + v.handles = append(v.handles, nil) + + return uintptr(slot) +} + +// Track adds the given pointer to the list of pointers to track and +// returns a pointer value which can be passed to C as an opaque +// pointer. +func (v *HandleList) Track(pointer interface{}) unsafe.Pointer { + v.Lock() + + slot := v.findUnusedSlot() + v.handles[slot] = pointer + v.set[slot] = true + + v.Unlock() + + return unsafe.Pointer(slot) +} + +// Untrack stops tracking the pointer given by the handle +func (v *HandleList) Untrack(handle unsafe.Pointer) { + slot := uintptr(handle) + + v.Lock() + + v.handles[slot] = nil + delete(v.set, slot) + + v.Unlock() +} + +// Get retrieves the pointer from the given handle +func (v *HandleList) Get(handle unsafe.Pointer) interface{} { + slot := uintptr(handle) + + v.RLock() + + if _, ok := v.set[slot]; !ok { + panic("invalid pointer handle") + } + + ptr := v.handles[slot] + + v.RUnlock() + + return ptr +} -- cgit v1.2.3 From bde012f3d478752787bab0978e0bfaf5deefa42b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Apr 2015 10:02:57 +0200 Subject: handles: correctly initialize all members --- handles.go | 1 + 1 file changed, 1 insertion(+) (limited to 'handles.go') diff --git a/handles.go b/handles.go index c0d1889..a862746 100644 --- a/handles.go +++ b/handles.go @@ -16,6 +16,7 @@ type HandleList struct { func NewHandleList() *HandleList { return &HandleList{ handles: make([]interface{}, 5), + set: make(map[uintptr]bool), } } -- cgit v1.2.3 From 0a336e4abdd7c949c9dd38037165e6a16c7d7ebf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 09:35:58 +0200 Subject: handles: start slot indices with 1 Using 0 as the first slot indice leads to not being able to differentiate between a handle to the first element or a NULL-handle. As current code may check whether the pointer is NULL, change the first indice to be 1 instead. --- handles.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'handles.go') diff --git a/handles.go b/handles.go index a862746..d173785 100644 --- a/handles.go +++ b/handles.go @@ -23,7 +23,7 @@ func NewHandleList() *HandleList { // findUnusedSlot finds the smallest-index empty space in our // list. You must only run this function while holding a write lock. func (v *HandleList) findUnusedSlot() uintptr { - for i := 0; i < len(v.handles); i++ { + for i := 1; i < len(v.handles); i++ { isUsed := v.set[uintptr(i)] if !isUsed { return uintptr(i) -- cgit v1.2.3 From 7ee534d0c50f6b7b54a6e8c83e0953a95a4cac22 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:08:32 +0200 Subject: handles: print pointer handle on panic. --- handles.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'handles.go') diff --git a/handles.go b/handles.go index d173785..1cbf6eb 100644 --- a/handles.go +++ b/handles.go @@ -1,6 +1,7 @@ package git import ( + "fmt" "sync" "unsafe" ) @@ -72,7 +73,7 @@ func (v *HandleList) Get(handle unsafe.Pointer) interface{} { v.RLock() if _, ok := v.set[slot]; !ok { - panic("invalid pointer handle") + panic(fmt.Sprintf("invalid pointer handle: %p", handle)) } ptr := v.handles[slot] -- cgit v1.2.3 From 1bd338af5e7a329c8ec5bd85500350795d0793d2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 09:50:16 +0200 Subject: handles: do not store handles by uintptr If we store values by uintptrs the GC may try to inspect their values when it kicks in. As the pointers are most likely invalid, this will result in an invalid pointer dereference, causing the program to panic. We can fix this by storing values by an int index value instead, returning pointers to those indices as handles instead. --- handles.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'handles.go') diff --git a/handles.go b/handles.go index 1cbf6eb..ec62a48 100644 --- a/handles.go +++ b/handles.go @@ -11,23 +11,23 @@ type HandleList struct { // stores the Go pointers handles []interface{} // indicates which indices are in use - set map[uintptr]bool + set map[int]bool } func NewHandleList() *HandleList { return &HandleList{ handles: make([]interface{}, 5), - set: make(map[uintptr]bool), + set: make(map[int]bool), } } // findUnusedSlot finds the smallest-index empty space in our // list. You must only run this function while holding a write lock. -func (v *HandleList) findUnusedSlot() uintptr { +func (v *HandleList) findUnusedSlot() int { for i := 1; i < len(v.handles); i++ { - isUsed := v.set[uintptr(i)] + isUsed := v.set[i] if !isUsed { - return uintptr(i) + return i } } @@ -36,7 +36,7 @@ func (v *HandleList) findUnusedSlot() uintptr { slot := len(v.handles) v.handles = append(v.handles, nil) - return uintptr(slot) + return slot } // Track adds the given pointer to the list of pointers to track and @@ -51,12 +51,12 @@ func (v *HandleList) Track(pointer interface{}) unsafe.Pointer { v.Unlock() - return unsafe.Pointer(slot) + return unsafe.Pointer(&slot) } // Untrack stops tracking the pointer given by the handle func (v *HandleList) Untrack(handle unsafe.Pointer) { - slot := uintptr(handle) + slot := *(*int)(handle) v.Lock() @@ -68,7 +68,7 @@ func (v *HandleList) Untrack(handle unsafe.Pointer) { // Get retrieves the pointer from the given handle func (v *HandleList) Get(handle unsafe.Pointer) interface{} { - slot := uintptr(handle) + slot := *(*int)(handle) v.RLock() -- cgit v1.2.3