Feature/pipeline explicit native release - #29
Open
IrishBAM wants to merge 7 commits into
Open
Conversation
Release the native GstPipeline eagerly instead of waiting for GC to finalize the wrapper, avoiding unbounded native memory growth when a pipeline is built per unit of work. dispose() drives the pipeline to NULL and drops the owning reference; a require_pipeline guard makes any use-after-dispose throw. Terminal and idempotent. Adds unit tests, README docs, and an example.
dispose() no longer issues a synchronous gst_element_set_state(NULL) on the JS thread — it just drops the owning reference via reset(), letting GStreamer tear the pipeline down when the last ref is released. Removes the event-loop blocking and the ignored state-change return raised in review. Document the stop()-and-release-elements-before-dispose contract and adjust the still-playing dispose test.
dispose() called pipeline.reset() without checking state. GStreamer refuses to tear down a non-NULL element, so disposing a running pipeline leaked the native memory instead of reclaiming it. Now query state and throw "dispose() requires a stopped pipeline" unless the pipeline is already NULL. Update tests to assert the still-playing case throws, add a worker-in-flight test, and correct the README and example that described the removed set_state(NULL) behaviour.
…prevents native leak)
…ndows Sending EOS to a running source and calling stop() immediately raced GStreamer's internal basesrc has_pending_eos handling, aborting the vitest worker fork on Windows CI (all JS assertions passed but the fork died). Add a waitForEos helper and await EOS on the bus before stop() in the affected EOS tests so the source streaming thread unwinds its loop cleanly before the state teardown.
play/pause/stop were identical except for the target state; extract a shared queue_state_change() helper so each is a one-liner. Pull the repeated timeout-parsing block into parse_timeout() (reused by bus_pop), and collapse the duplicated throw in require_pipeline() into a single condition. No behavior change.
Drop the waitForEos helper and the EOS test changes that used it, keeping this branch scoped to the dispose() feature. The Windows basesrc flake fix belongs in its own branch/PR.
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.
Add
Pipeline.dispose()for explicit native releaseWHAT
Adds a
dispose()method toPipelinethat releases the underlying nativeGstPipelineimmediately, plus a use-after-dispose guard, TypeScript type,documentation, an example, and unit tests.
src/cpp/pipeline.cpp/pipeline.hpp— newdispose(); drops the owningreference (
unique_ptr::reset()→gst_object_unref) without issuing a statechange. Adds a
require_pipeline()guard so every method throwsPipeline used after dispose()instead of dereferencing a freed pointer.src/ts/index.ts—dispose(): voidon thePipelineinterface.src/ts/pipeline-dispose.test.ts— coverage for idempotency and theuse-after-dispose guard (sync + async methods).
README.md/examples/dispose.mjs— usage, rationale, and the disposalcontract.
WHY
The native
GstPipelineallocation (buffers, decoders, GStreamer internals)lives outside V8's heap and is invisible to its GC accounting. A pipeline that
is simply dropped is only reclaimed when GC happens to collect the small JS
wrapper — and with a flat JS heap, V8 feels little pressure to do so. A
long-running process that builds a pipeline per unit of work (recording,
transcode, feed) can watch RSS climb steadily while the JS heap stays flat.
There was previously no way to release the native pipeline eagerly.
HOW
dispose()drops the wrapper's owning reference; GStreamer tears the pipelinedown to NULL when the last reference is released. It does not issue a
synchronous
gst_element_set_state(NULL), which would block the JS thread —callers
stop()first (the documented contract), matching howplay/pause/stopoffload transitions to a worker.BusPopWorker/StateChangeWorkerinstances each hold their own
gst_object_ref(seeasync-workers.cpp), sodropping the wrapper's reference cannot free the pipeline out from under a
running
busPop()/state change.dispose()is a no-op; any other methodcall afterward throws via the
require_pipeline()guard.stop()(orendOfStream()+ wait for EOS, thenstop()) beforedispose(), and release anygetElementByName()elements /pad-probe /
onSample()subscriptions first, since those hold independentreferences and are not invalidated by disposing the pipeline.
Testing
npm run build(native + TS) andnpm run lintclean.