From fc188d008db8b0de4e236b59f4175382fbbd2b4d Mon Sep 17 00:00:00 2001 From: Alexander Petrov Date: Thu, 30 Jul 2020 00:24:25 +0200 Subject: [PATCH] issue-433 avoid keeping all the allocated handles forever by using map and uintptr of the allocated object. --- c-bindings/c-bindings.go | 14 ++++----- c-bindings/handles.go | 65 ++++++++++++++------------------------- c-bindings/internal.h | 8 ++--- c-bindings/libjsonnet.cpp | 4 +-- 4 files changed, 36 insertions(+), 55 deletions(-) diff --git a/c-bindings/c-bindings.go b/c-bindings/c-bindings.go index 6a6a812..108c4da 100644 --- a/c-bindings/c-bindings.go +++ b/c-bindings/c-bindings.go @@ -82,7 +82,7 @@ func (i *importer) Import(importedFrom, importedPath string) (contents jsonnet.C return i.contentCache[foundHere], foundHere, nil } -var handles = handlesTable{} +var handles = newHandlesTable() var versionString *C.char //export jsonnet_version @@ -106,12 +106,12 @@ func jsonnet_make() *C.struct_JsonnetVm { os.Exit(1) } - return C.jsonnet_internal_make_vm_with_id(C.uint64_t(id)) + return C.jsonnet_internal_make_vm_with_id(C.uintptr_t(id)) } //export jsonnet_destroy func jsonnet_destroy(vmRef *C.struct_JsonnetVm) { - if err := handles.free(uint64(vmRef.id)); err != nil { + if err := handles.free(uintptr(vmRef.id)); err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) } @@ -120,7 +120,7 @@ func jsonnet_destroy(vmRef *C.struct_JsonnetVm) { } func getVM(vmRef *C.struct_JsonnetVm) *vm { - ref, err := handles.get(uint64(vmRef.id)) + ref, err := handles.get(uintptr(vmRef.id)) if err != nil { fmt.Fprintln(os.Stderr, err.Error()) @@ -411,7 +411,7 @@ func jsonnet_json_destroy(vmRef *C.struct_JsonnetVm, v *C.struct_JsonnetJsonValu jsonnet_json_destroy(vmRef, child) } - if err := handles.free(uint64(v.id)); err != nil { + if err := handles.free(uintptr(v.id)); err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) } @@ -427,11 +427,11 @@ func createJSONValue(vmRef *C.struct_JsonnetVm, val interface{}) *C.struct_Jsonn os.Exit(1) } - return C.jsonnet_internal_make_json_with_id(C.uint64_t(id)) + return C.jsonnet_internal_make_json_with_id(C.uintptr_t(id)) } func getJSONValue(jsonRef *C.struct_JsonnetJsonValue) *jsonValue { - ref, err := handles.get(uint64(jsonRef.id)) + ref, err := handles.get(uintptr(jsonRef.id)) if err != nil { fmt.Fprintln(os.Stderr, err.Error()) diff --git a/c-bindings/handles.go b/c-bindings/handles.go index 00da2c4..f2a92aa 100644 --- a/c-bindings/handles.go +++ b/c-bindings/handles.go @@ -2,11 +2,9 @@ package main import ( "errors" - "fmt" + "unsafe" ) -const maxID = (1 << 63) - 1 - // Because of Go GC, there are restrictions on keeping Go pointers in C. // We cannot just pass *jsonnet.VM/JsonValue to C. So instead we use "handle" structs in C // which refer to JsonnetVM/JsonnetJsonValue by a numeric id. @@ -17,62 +15,45 @@ const maxID = (1 << 63) - 1 // handlesTable is the set of active, valid Jsonnet allocated handles type handlesTable struct { - objects []interface{} - freedIDs []uint64 + handles map[uintptr]*handle } -// errMaxNumberOfOpenHandles tells that there was an attempt to create more than maxID open handles -var errMaxNumberOfOpenHandles = fmt.Errorf("maximum number of constructed Jsonnet handles exceeded (%d)", maxID) +type handle struct { + ref interface{} +} // errInvalidHandle tells that there was an attempt to dereference invalid handle ID var errInvalidHandle = errors.New("invalid handle ID was provided") -// make registers the new object as a handle and returns the corresponding ID -func (h *handlesTable) make(obj interface{}) (uint64, error) { - var id uint64 - - if len(h.freedIDs) > 0 { - id, h.freedIDs = h.freedIDs[len(h.freedIDs)-1], h.freedIDs[:len(h.freedIDs)-1] - h.objects[id-1] = obj - } else { - id = uint64(len(h.objects) + 1) - - if id > maxID { - return 0, errMaxNumberOfOpenHandles - } - - h.objects = append(h.objects, obj) +func newHandlesTable() handlesTable { + return handlesTable{ + handles: make(map[uintptr]*handle), } +} +// make registers the new object as a handle and returns the corresponding ID +func (h *handlesTable) make(obj interface{}) (uintptr, error) { + entry := &handle{ref: obj} + id := uintptr(unsafe.Pointer(entry)) + h.handles[id] = entry return id, nil } -// free marks the given handle ID as unused -func (h *handlesTable) free(id uint64) error { - if err := h.ensureValidID(id); err != nil { - return err +// free removes an object with the given ID +func (h *handlesTable) free(id uintptr) error { + if _, ok := h.handles[id]; !ok { + return errInvalidHandle } - h.objects[id-1] = nil - h.freedIDs = append(h.freedIDs, id) - + delete(h.handles, id) return nil } // get returns the corresponding object for the provided ID -func (h *handlesTable) get(id uint64) (interface{}, error) { - if err := h.ensureValidID(id); err != nil { - return nil, err +func (h *handlesTable) get(id uintptr) (interface{}, error) { + if _, ok := h.handles[id]; !ok { + return nil, errInvalidHandle } - return h.objects[id-1], nil -} - -// ensureValidID returns an error if the given handle ID is invalid, otherwise returns nil -func (h *handlesTable) ensureValidID(id uint64) error { - if id == 0 || uint64(id) > uint64(len(h.objects)) { - return errInvalidHandle - } - - return nil + return h.handles[id].ref, nil } diff --git a/c-bindings/internal.h b/c-bindings/internal.h index f8d54a2..b28530b 100644 --- a/c-bindings/internal.h +++ b/c-bindings/internal.h @@ -3,17 +3,17 @@ #include struct JsonnetVm { - uint64_t id; + uintptr_t id; }; -struct JsonnetVm *jsonnet_internal_make_vm_with_id(uint64_t id); +struct JsonnetVm *jsonnet_internal_make_vm_with_id(uintptr_t id); void jsonnet_internal_free_vm(struct JsonnetVm *x); struct JsonnetJsonValue { - uint64_t id; + uintptr_t id; }; -struct JsonnetJsonValue *jsonnet_internal_make_json_with_id(uint64_t id); +struct JsonnetJsonValue *jsonnet_internal_make_json_with_id(uintptr_t id); void jsonnet_internal_free_json(struct JsonnetJsonValue *x); typedef struct JsonnetJsonValue *JsonnetNativeCallback(void *ctx, diff --git a/c-bindings/libjsonnet.cpp b/c-bindings/libjsonnet.cpp index 48b070b..303f9a2 100644 --- a/c-bindings/libjsonnet.cpp +++ b/c-bindings/libjsonnet.cpp @@ -6,7 +6,7 @@ extern "C" { #include "internal.h" } -struct JsonnetVm *jsonnet_internal_make_vm_with_id(uint64_t id) { +struct JsonnetVm *jsonnet_internal_make_vm_with_id(uintptr_t id) { JsonnetVm *vm = new JsonnetVm(); vm->id = id; return vm; @@ -16,7 +16,7 @@ void jsonnet_internal_free_vm(struct JsonnetVm *x) { delete(x); } -struct JsonnetJsonValue *jsonnet_internal_make_json_with_id(uint64_t id) { +struct JsonnetJsonValue *jsonnet_internal_make_json_with_id(uintptr_t id) { JsonnetJsonValue *json = new JsonnetJsonValue(); json->id = id; return json;