Skip to content

Stop __del__ raising on objects whose construction failed - #93

Merged
birkholz-cubert merged 1 commit into
developfrom
feature/handle-lifecycle-and-capture
Aug 19, 2026
Merged

Stop __del__ raising on objects whose construction failed#93
birkholz-cubert merged 1 commit into
developfrom
feature/handle-lifecycle-and-capture

Conversation

@birkholz-cubert

@birkholz-cubert birkholz-cubert commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

What

  • Guards all ten __del__ implementations against a handle that was never assigned. Seven of the eight testable handle owners previously raised TypeError out of garbage collection after any failed construction.
  • AcquisitionContext.capture(to_internal=True) now reaches the SDK. It passed a Python 0 where SWIG wants an int32_t*, so it raised before the call and the internal queue was unreachable from Python.
  • Renames the to_interal parameter to to_internal, with no alias for the old spelling.
  • Adds tests/test_handle_lifecycle.py plus three capture tests.

Why

  • Every handle owner sets self._handle = None before the call that fills it in, so a constructor that raises leaves a half-built object for the collector. Python prints rather than propagates exceptions from __del__, so the symptom was an unrelated looking traceback appearing after an error the caller had already handled.
  • cuvis.h:1886 documents a null result handle as the way to queue a capture internally, so the intent was right and only the marshalling was wrong. SWIG will not coerce a Python int to a null pointer, but it does marshal None.
  • The next release is a major version, and correcting a typo is not a feature removal, so the misspelling is simply gone rather than carried through a deprecation cycle.

The queue switch, verified in both directions

cuvis.h:1887 names two consumers of the internal queue, cuvis_acq_cont_get_next_measurement and the worker. Both were checked, with the queue proved empty beforehand so nothing else can be mistaken for the capture under test.

  • to_internal=True returns None, and the measurement arrives at the direct consumer and at a Worker with set_acquisition_context. The result is processed to a (275, 290, 51) cube to prove it is a usable measurement and not a handle the SDK never filled in. The queue is empty again afterwards, so exactly one measurement was produced.
  • to_internal=False returns an AsyncMesu and puts nothing in the queue. The async result is collected first, so the capture is known to have happened; neither consumer then sees anything. This is the half of the ALL-5828 report that does not reproduce, now pinned by a regression test rather than changed.

Pitfalls

  • Breaking for anyone passing to_interal= by keyword; they get TypeError: capture() got an unexpected keyword argument 'to_interal'. Positional callers are unaffected. This wants to land in the major release it assumes.
  • The capture tests use the simulated camera fixture, so they are sensitive to queue state left behind by neighbouring tests. Each asserts the queue is empty on entry and drains what it queues.

Verification

  • 134 passed against the installed SDK, up from 123.
  • The eight new lifecycle tests fail 8/8 on develop and pass 8/8 here.
  • Rebased onto develop after Require Python 3.10 and use the annotations it provides #94; no legacy Optional/Union annotations remain, confirmed with ruff check --select UP006,UP007,UP035,UP045.
  • ruff check, ruff format --check and scripts/check_changelog.py clean.

@birkholz-cubert
birkholz-cubert force-pushed the feature/handle-lifecycle-and-capture branch 2 times, most recently from 92cea29 to 3d44353 Compare August 19, 2026 16:37
Every SDK handle owner sets self._handle to None before the call that
would fill it in, so a constructor that raises leaves a half-built
object behind. __del__ then handed that None to the SWIG free function,
which raised TypeError. Python does not propagate out of __del__, it
prints, so the symptom was an unrelated looking traceback surfacing
after an error the caller had already handled. Ten sites, guarded
uniformly; seven of the eight testable classes were affected.

Two ride along, both in AcquisitionContext.capture. The to_internal
path passed a Python 0 where SWIG wants an int32_t*, so it raised
before reaching the SDK and the internal queue documented at
cuvis.h:1886 was unreachable from Python; SWIG marshals None to NULL.
The parameter is also renamed off its misspelling. No alias is kept for
to_interal: the next release is a major version, and a rename is not a
feature removal, so callers get a plain TypeError rather than a
deprecation cycle for a typo.
@birkholz-cubert
birkholz-cubert force-pushed the feature/handle-lifecycle-and-capture branch from 3d44353 to cb79622 Compare August 19, 2026 17:15
@birkholz-cubert
birkholz-cubert merged commit 5393a93 into develop Aug 19, 2026
4 checks passed
@birkholz-cubert
birkholz-cubert deleted the feature/handle-lifecycle-and-capture branch August 19, 2026 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant