Add OwnedBytes to transfer owned buffers through djinni, use for Lines#916
Add OwnedBytes to transfer owned buffers through djinni, use for Lines#916
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1e7b7383a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| std::free((void*)bytes.address); | ||
| return std::make_pair(nullptr, 0); | ||
| } | ||
| return std::make_pair(std::unique_ptr<T>(reinterpret_cast<T*>(bytes.address)), bytes.elementCount); |
There was a problem hiding this comment.
toUniquePtr wraps a buffer containing elementCount elements in std::unique_ptr<T> with the default single-object deleter, so normal line buffers (elementCount > 1) are released with scalar delete even though they were allocated as a contiguous block via the vector allocator. This is undefined behavior (ASan reports new-delete-type-mismatch) and can corrupt heap state when LineGroup2dOpenGl::setLines drops or replaces these buffers.
Useful? React with 👍 / 👎.
| guard let pointer = UnsafeRawPointer(bitPattern: Int(ownedBytes.address)), | ||
| ownedBytes.elementCount > 0 | ||
| else { return nil } |
There was a problem hiding this comment.
This guard returns nil without calling MCOwnedBytesDestructor.free, but MCOwnedBytes is ownership-transferring and must always be released. A reachable case is OwnedBytesHelper::fromVector producing elementCount == 0 with a non-null address when a vector has reserved capacity but no emitted vertices (for example degenerate lines), which causes repeated native-memory leaks on iOS in this branch.
Useful? React with 👍 / 👎.
matzf
force-pushed
the
feature/owned-bytes
branch
4 times, most recently
from
4ac02b9 to
12e7e66
Compare
OwnedBytes transfers ownership over buffer to callee. When received in C++, convert back to a unique_ptr; otherwise, e.g. in swift, destroy with OwnedBytesDestructer::free.
matzf
force-pushed
the
feature/owned-bytes
branch
from
12e7e66 to
a847021
Compare
1 participant
OwnedBytes transfers ownership over buffer to callee.
When received in C++, convert back to a unique_ptr; otherwise, e.g. in
swift, destroy with OwnedBytesDestructer::free.
Use for setLines, avoid one copy for OpenGL. Note that the line buffers can be are very large on e.g. swisstopo basemap (very roughly on the order of a hundred megabytes).