Generic safe value-vector conversion (KeyPoint + DMatch)#90
Merged
Conversation
The generic cpp_to_julia(StdVector{T}) returns a Vector{TDereferenced} whose
elements are views into the C++ vector. Once the source std::vector is freed
those views dangle (use-after-free), and the Dereferenced element type is
invariant-incompatible with the generated signatures (which want Vector{T}).
KeyPoint was patched ad hoc; DMatch (match/knnMatch/radiusMatch) had the same
latent bug, masked only because tests called length() without dereferencing.
Generalize into one place (types_conversion.jl): a _copy_cxx_value registry
that value-copies each element into a fresh, owned object while the source is
alive, with cpp_to_julia(StdVector{T})/julia_to_cpp(Array{<:T,1}) generated for
T in (KeyPoint, DMatch). Nested Vector{Vector{DMatch}} (knnMatch) is covered via
the generic outer path. Adding a type is now one tuple entry.
Tests (test_descriptormatcher.jl + test_feature2d.jl): DMatch match/knnMatch
field reads after GC.gc(true) (use-after-free regression guard), julia_to_cpp
round-trip integrity, KeyPoint GC-stress between detect and compute, and direct
_copy_cxx_value field-preservation checks. 230/230 pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OpenCV_jll pins libcxxwrap_julia_jll = 0.14.7, which requires CxxWrap >= 0.17.4 (registry C/CxxWrap/Compat.toml). The old "0.16, 0.17" left 0.16 admissible — and 0.16 needs libcxxwrap 0.13, so in any resolve where the JLL does not transitively force libcxxwrap the solver could pick 0.16 and hit the 'invalid subtyping StdString/CppBasicString' load crash. Drop the dead 0.16. Verified: clean resolve picks CxxWrap 0.17.5; full suite 230/230. The 'min' CI job (Julia 1.10) still resolves (0.17.4 supports Julia >= 1.6 and libcxxwrap 0.14.7). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 5, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #90 +/- ##
==========================================
+ Coverage 9.20% 11.79% +2.59%
==========================================
Files 18 18
Lines 2086 2594 +508
==========================================
+ Hits 192 306 +114
- Misses 1894 2288 +394 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Tighten CxxWrap compat to 0.17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Generalizes the recently-applied KeyPoint
detect→computefix into a single, extensible conversion forstd::vector<value-type>, and closes the identical latent use-after-free in DMatch (match/knnMatch/radiusMatch).The bug class
The generic
cpp_to_julia(StdVector{T})returnsVector{TDereferenced}whose elements are views into the C++ vector. Once the sourcestd::vectoris freed those views dangle (use-after-free), and theDereferencedelement type is invariant-incompatible with the generated signatures (which wantVector{T}). DMatch was safe only because tests calledlength()without ever dereferencing an element — readingmatches[i].distanceafter a GC would have read freed memory.Change
Moved the keypoint conversion out of
cv_manual_wrap.jlintotypes_conversion.jland generalized via a small registry:_copy_cxx_value(::KeyPoint)/_copy_cxx_value(::DMatch)value-copy one element into a fresh, owned object while the source is alive.for T in (KeyPoint, DMatch)loop emitscpp_to_julia(StdVector{T})andjulia_to_cpp(Array{<:T,1}). NestedVector{Vector{DMatch}}(knnMatch) is covered via the generic outer path. Adding a type is one tuple entry.Tests
New
test/test_descriptormatcher.jl+ additions totest/test_feature2d.jl:match/knnMatchfield reads afterGC.gc(true)(use-after-free regression guard).julia_to_cpp→cpp_to_juliaround-trip integrity.detectandcompute;_copy_cxx_valuefield-preservation.Full suite: 230/230 pass (was 208) against a locally-built
OpenCV_jll 4.13.0.🤖 Generated with Claude Code