Uh oh!
There was an error while loading. Please reload this page.
fix: Centralize ManagedResources lifecycle handling - #294
Conversation
* fix: Docs * fix: Error handling memory * fix: Update the docs * fix: Refactor * fix: Fix script
Uh oh!
There was an error while loading. Please reload this page.
* fix: Refactor * fix: Refactor * fix: Typo * fix: refactor * fix: Refactor * fix: Refactor 2 * fix: Clean up debug
…-api' into mathern/open-up-api
ale-adobe
left a comment
There was a problem hiding this comment.
LGTM, all comments and thoughts are non-blocking.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
gpeacock
left a comment
There was a problem hiding this comment.
It feels like a failure of the c_ffi_bindings that it is this complex to track and handle things. I'd like to track what we can do to make this less of a burden on the bindings.
- a better/ thread safe and atomic way to return error strings
- consider using unique identifiers instead of actual reusable memory adresses.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Changes in this pull request
Centralizes ManagedResources lifecycle plumbing (the managed resources holding a native handle) natively and handle their lifecycle. This change introduces a set of new (private) functions on the ManagedResource class for lifecycle management. Some notable additions are:
_activate(handle): take ownership of a validated non-null pointer and mark the resource active. Rejects null and double-activation, so a handle can't be activated twice and a closed resource can't be reopened._swap_handle(new_handle): replace the handle after an FFI call consumed the old one and returned a replacement._consume_*(family of functions): runs an FFI call that consumes this handle and manages the handle lifecycle as part of the object's lifecycle._wrap_native_handle(handle): build an instance around an already-owned pointer, bypassing init._init_attrs(): subclass hook for attribute defaults, called by both init and _wrap_native_handle so an instance built around an existing handle can't be missing state the class reads.Those are all private APIs for native handling (hence underscore in function name, using and calling them must be very explicit).
Note on fork safety: the owner PID is still stamped on all paths, and the native free is skipped in a foreign process on all teardown paths.
Other changes:
Checklist
TO DOitems (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.