Skip to content

Segment: retain chunk ownership when allocation fails - #138

Open
victorstoica114 wants to merge 1 commit into
sigrokproject:masterfrom
victorstoica114:fix-segment-allocation-ownership
Open

victorstoica114 wants to merge 1 commit into
sigrokproject:masterfrom
victorstoica114:fix-segment-allocation-ownership

Conversation

@victorstoica114

Copy link
Copy Markdown

When tail compaction removes and reinserts the last deque entry, deque-node allocation can fail after the original chunk has been freed. Bulk append can also delete an already-owned chunk when successor allocation fails, or leak its successor if deque insertion fails. Single-sample append has the same insertion ownership gap.

Replace the existing tail pointer in place. Hold each successor in unique_ptr until its deque entry is inserted, then publish it as the current chunk. The temporary reserve allocation, sample-count behavior and exception propagation are retained. Previously accepted samples remain readable after these failures; this does not add retry-after-failed-append semantics or constructor rollback.

The standalone segment-allocation-test runs only with ENABLE_TESTS. Its allocation overrides stay out of PulseView and the existing Boost executable. It covers 31 compaction/bulk/single append cases, including deque boundaries, allocation failure, retained-prefix equality and successor leaks.

Validation against upstream af02198741b4e57c9f9b796bd5e6c0f2ae9f2f2b:

  • Full Release builds and both CTest entries pass on Linux and native Windows/MinGW. The new regression reports 31 cases, zero failures on both. Original code reproduces the ownership failures; separate negative controls isolate bulk allocation, bulk insertion and single insertion failures.
  • Two bounded sequential DLA-32 captures, one per OS: 5 MS/s, 32 channels, 25 million samples / 100 MB. A private bridge feeds every original libsigrok LOGIC packet into this exact Segment source and unchanged upstream header, then completes, compacts and reads its storage. The original feed, stored bytes and independent RAW conversion match exactly. Both retain all 50 fresh consecutive CRC UART messages, agree between two UART decoders, and have no out-of-range D0 clock periods. Stop/END/close succeed; original RAW tails are retained.
  • Linux runs under VMware; Windows is a nested KVM guest. The electrical witnesses cover D0/D22, not arbitrary signals on every channel. These are separate allocation-failure and real-data tests, not a claim of global OOM recovery or simultaneous electrical/fault injection. This PR does not change the acquisition driver or qualify the separate Windows user-desktop workflow.

This is a focused three-file change based directly on upstream master; it does not depend on #136 or #137. Private captures, the bridge and vendor runtimes are not included.

Keep the compacted tail in its existing deque entry, and retain successor chunks in unique_ptr until deque insertion succeeds. Preserve the existing reserve allocation and exception behavior.

Add an isolated 31-case CTest regression for compaction and bulk/single append allocation failures, without allocation hooks in the application.
Copilot AI balanced review requested due to automatic review settings November 4, 2026 04:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

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.

2 participants