Merge pull request #26 from andrewwhitehead/fix/python-keepalive

(python) Update object finalizers and add keepalives

wrappers/python/indy_credx/bindings.py

@@ -9,7 +9,10 @@
     CDLL,
     POINTER,
     Structure,
+    addressof,
     byref,
+    cast,
+    c_char,
     c_char_p,
     c_int8,
     c_int64,
@@ -20,7 +23,8 @@
 )
 from ctypes.util import find_library
 from io import BytesIO
-from typing import Optional, Mapping, Sequence, Tuple, Union
+from typing import Callable, Optional, Mapping, Sequence, Tuple, Union
+from weakref import finalize
 
 from .error import CredxError, CredxErrorCode
 
@@ -33,9 +37,40 @@
 JsonType = Union[dict, str, bytes, memoryview]
 
 
-class ObjectHandle(c_int64):
+def _struct_dtor(ctype: type, address: int, dtor: Callable):
+    value = ctype.from_address(address)
+    if value:
+        dtor(value)
+
+
+def finalize_struct(instance, ctype):
+    """Attach a struct destructor."""
+    finalize(
+        instance, _struct_dtor, ctype, addressof(instance), instance.__class__._cleanup
+    )
+
+
+def keepalive(instance, *depend):
+    """Ensure that dependencies are kept alive as long as the instance."""
+    finalize(instance, lambda *_args: None, *depend)
+
+
+class ObjectHandle(Structure):
     """Index of an active IndyObject instance."""
 
+    _fields_ = [
+        ("value", c_int64),
+    ]
+
+    def __init__(self, value=0):
+        """Initializer."""
+        if isinstance(value, c_int64):
+            value = value.value
+        if not isinstance(value, int):
+            raise ValueError("Invalid handle")
+        super().__init__(value=value)
+        finalize_struct(self, c_int64)
+
     @property
     def type_name(self) -> str:
         return object_get_type_name(self)
@@ -51,8 +86,10 @@ def __repr__(self) -> str:
             type_name = "<none>"
         return f"{self.__class__.__name__}({type_name}, {self.value})"
 
-    def __del__(self):
-        object_free(self)
+    @classmethod
+    def _cleanup(cls, value: c_int64):
+        """Destructor."""
+        get_library().credx_object_free(value)
 
 
 class IndyObject:
@@ -78,65 +115,118 @@ def to_json(self) -> str:
         return bytes(object_get_json(self.handle)).decode("utf-8")
 
     def to_json_buffer(self) -> memoryview:
-        return memoryview(object_get_json(self.handle).raw)
+        return object_get_json(self.handle).raw
 
 
-class ByteBuffer(Structure):
+class RawBuffer(Structure):
     """A byte buffer allocated by the library."""
 
     _fields_ = [
         ("len", c_int64),
-        ("value", c_void_p),
+        ("data", POINTER(c_ubyte)),
     ]
 
+    def __bool__(self) -> bool:
+        return bool(self.data)
+
+    def __bytes__(self) -> bytes:
+        if not self.len:
+            return b""
+        return bytes(self.array)
+
+    def __len__(self) -> int:
+        return int(self.len)
+
     @property
-    def raw(self) -> Array:
-        ret = (c_ubyte * self.len).from_address(self.value)
-        setattr(ret, "_ref_", self)  # ensure buffer is not dropped
-        return ret
+    def array(self) -> Array:
+        return cast(self.data, POINTER(c_ubyte * self.len)).contents
+
+    def __repr__(self) -> str:
+        return f"<RawBuffer(len={self.len})>"
+
+
+class ByteBuffer(Structure):
+    """A managed byte buffer allocated by the library."""
+
+    _fields_ = [("buffer", RawBuffer)]
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        finalize_struct(self, RawBuffer)
+
+    @property
+    def _as_parameter_(self):
+        return self.buffer
+
+    @property
+    def array(self) -> Array:
+        return self.buffer.array
+
+    @property
+    def raw(self) -> memoryview:
+        m = memoryview(self.array)
+        keepalive(m, self)
+        return m
 
     def __bytes__(self) -> bytes:
-        return bytes(self.raw)
+        return bytes(self.buffer)
+
+    def __len__(self) -> int:
+        return len(self.buffer)
+
+    def __getitem__(self, idx) -> bytes:
+        return bytes(self.buffer.array[idx])
 
     def __repr__(self) -> str:
         """Format byte buffer as a string."""
-        return repr(bytes(self))
+        return f"{self.__class__.__name__}({bytes(self)})"
 
-    def __del__(self):
+    @classmethod
+    def _cleanup(cls, buffer: RawBuffer):
         """Call the byte buffer destructor when this instance is released."""
-        get_library().credx_buffer_free(self)
+        get_library().credx_buffer_free(buffer)
 
 
-class StrBuffer(c_char_p):
+class StrBuffer(Structure):
     """A string allocated by the library."""
 
-    @classmethod
-    def from_param(cls):
-        """Returns the type ctypes should use for loading the result."""
-        return c_void_p
+    _fields_ = [("buffer", POINTER(c_char))]
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        finalize_struct(self, c_char_p)
 
     def is_none(self) -> bool:
         """Check if the returned string pointer is null."""
-        return self.value is None
+        return not self.buffer
 
     def opt_str(self) -> Optional[str]:
         """Convert to an optional string."""
         val = self.value
         return val.decode("utf-8") if val is not None else None
 
+    def __bool__(self) -> bool:
+        return bool(self.buffer)
+
     def __bytes__(self) -> bytes:
         """Convert to bytes."""
-        return self.value
+        bval = self.value
+        return bval if bval is not None else bytes()
 
     def __str__(self):
         """Convert to a string."""
         # not allowed to return None
         val = self.opt_str()
         return val if val is not None else ""
 
-    def __del__(self):
+    @property
+    def value(self) -> bytes:
+        return cast(self.buffer, c_char_p).value
+
+    @classmethod
+    def _cleanup(cls, buffer: c_char_p):
         """Call the string destructor when this instance is released."""
-        get_library().credx_string_free(self)
+        get_library().credx_string_free(buffer)
 
 
 class FfiObjectHandleList(Structure):
@@ -197,15 +287,17 @@ class CredentialEntry(Structure):
     @classmethod
     def create(
         cls,
-        credential: ObjectHandle,
+        credential: IndyObject,
         timestamp: int = None,
-        rev_state: ObjectHandle = None,
+        rev_state: IndyObject = None,
     ) -> "CredentialEntry":
-        return CredentialEntry(
-            credential=credential,
+        entry = CredentialEntry(
+            credential=credential.handle,
             timestamp=-1 if timestamp is None else timestamp,
-            rev_state=rev_state or ObjectHandle(),
+            rev_state=rev_state.handle if rev_state else ObjectHandle(),
         )
+        keepalive(entry, credential, rev_state)
+        return entry
 
 
 class CredentialEntryList(Structure):
@@ -271,42 +363,46 @@ class RevocationConfig(Structure):
     @classmethod
     def create(
         cls,
-        rev_reg_def: ObjectHandle,
-        rev_reg_def_private: ObjectHandle,
-        rev_reg: ObjectHandle,
+        rev_reg_def: IndyObject,
+        rev_reg_def_private: IndyObject,
+        rev_reg: IndyObject,
         rev_reg_index: int,
         rev_reg_used: Sequence[int],
         tails_path: str,
     ) -> "RevocationConfig":
-        return RevocationConfig(
-            rev_reg_def=rev_reg_def,
-            rev_reg_def_private=rev_reg_def_private,
-            rev_reg=rev_reg,
+        config = RevocationConfig(
+            rev_reg_def=rev_reg_def.handle,
+            rev_reg_def_private=rev_reg_def_private.handle,
+            rev_reg=rev_reg.handle,
             rev_reg_index=rev_reg_index,
             rev_reg_used=FfiIntList.create(rev_reg_used),
             tails_path=encode_str(tails_path),
         )
+        keepalive(config, rev_reg_def, rev_reg_def_private, rev_reg)
+        return config
 
 
 class RevocationEntry(Structure):
     _fields_ = [
         ("def_entry_idx", c_int64),
-        ("entry", ObjectHandle),
+        ("registry", ObjectHandle),
         ("timestamp", c_int64),
     ]
 
     @classmethod
     def create(
         cls,
         def_entry_idx: int,
-        entry: ObjectHandle,
+        registry: IndyObject,
         timestamp: int,
     ) -> "RevocationEntry":
-        return RevocationEntry(
+        entry = RevocationEntry(
             def_entry_idx=def_entry_idx,
-            entry=entry,
+            registry=registry.handle,
             timestamp=timestamp,
         )
+        keepalive(entry, registry)
+        return entry
 
 
 class RevocationEntryList(Structure):
@@ -442,10 +538,6 @@ def encode_bytes(arg: Optional[Union[str, bytes]]) -> FfiByteBuffer:
     return buf
 
 
-def object_free(handle: ObjectHandle):
-    get_library().credx_object_free(handle)
-
-
 def object_get_json(handle: ObjectHandle) -> ByteBuffer:
     result = ByteBuffer()
     do_call("credx_object_get_json", handle, byref(result))

wrappers/python/indy_credx/types.py

@@ -393,7 +393,7 @@ def create(
                 entry_idx = len(creds)
                 creds.append(
                     bindings.CredentialEntry.create(
-                        cred.handle, timestamp, rev_state and rev_state.handle
+                        cred, timestamp, rev_state and rev_state
                     )
                 )
                 for (reft, reveal) in attrs:
@@ -453,12 +453,12 @@ def verify(
                 reg_def = RevocationRegistryDefinition.load(reg_def)
             reg_def_id = reg_def.id
             if rev_reg_entries and reg_def_id in rev_reg_entries:
-                for timestamp, entry in rev_reg_entries[reg_def_id].items():
-                    if not isinstance(entry, bindings.IndyObject):
-                        entry = RevocationRegistry.load(entry)
+                for timestamp, registry in rev_reg_entries[reg_def_id].items():
+                    if not isinstance(registry, bindings.IndyObject):
+                        registry = RevocationRegistry.load(registry)
                     reg_entries.append(
                         bindings.RevocationEntry.create(
-                            len(reg_defs), entry.handle, timestamp
+                            len(reg_defs), registry, timestamp
                         )
                     )
             reg_defs.append(reg_def.handle)
@@ -661,9 +661,9 @@ def __init__(
     @property
     def _native(self) -> bindings.RevocationConfig:
         return bindings.RevocationConfig.create(
-            self.rev_reg_def.handle,
-            self.rev_reg_def_private.handle,
-            self.rev_reg.handle,
+            self.rev_reg_def,
+            self.rev_reg_def_private,
+            self.rev_reg,
             self.rev_reg_index,
             self.rev_reg_used,
             self.tails_path,