diff --git a/docs/go-conventions.md b/docs/go-conventions.md index 70ee02e72..cb5d81a82 100644 --- a/docs/go-conventions.md +++ b/docs/go-conventions.md @@ -97,3 +97,11 @@ contexts before returning. The leak helper waits for finalizer passes before requesting the native snapshot; keep that barrier synchronous and never block the finalizer queue, which also releases Go-owned GStreamer references. Verify changes to the helper with race-enabled media tests without disabling leak assertions. + +`WriterNewSample` releases each appsink sample when its callback returns, after +unmapping its buffer. The go-gst v1.4 `GstSample` wrapper contains only a pointer +to an opaque C type; on amd64 it is an 8-byte, GC-pointer-free allocation. Go's +tiny allocator can pack it beside another live object, preventing its finalizer +from running even after repeated GC. Clear the binding's finalizer before +explicitly releasing that transferred reference, or it may be released twice. +This is sample ownership, not a requirement to wrap every `Buffer.Map` call. diff --git a/pkg/media/io_helpers.go b/pkg/media/io_helpers.go index 886f58f97..357827c06 100644 --- a/pkg/media/io_helpers.go +++ b/pkg/media/io_helpers.go @@ -4,6 +4,7 @@ import ( "context" "errors" "io" + "runtime" "github.com/go-gst/go-gst/gst" "github.com/go-gst/go-gst/gst/app" @@ -86,6 +87,11 @@ func WriterNewSample(ctx context.Context, output io.Writer) func(sink *app.Sink) if sample == nil { return gst.FlowOK } + // GstSample is an opaque C type, so this small wrapper can share a + // Go tiny-allocation block with a live object and never be finalized. + // Release our owned reference when the callback finishes instead. + runtime.SetFinalizer(sample, nil) + defer sample.Unref() // Retrieve the buffer from the sample. buffer := sample.GetBuffer() diff --git a/pkg/media/io_helpers_test.go b/pkg/media/io_helpers_test.go index 027fda13c..4b5aac004 100644 --- a/pkg/media/io_helpers_test.go +++ b/pkg/media/io_helpers_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "runtime" "testing" "github.com/go-gst/go-gst/gst" @@ -42,6 +43,28 @@ func TestWriterNewSample(t *testing.T) { }) } +// A live tiny allocation can pin a neighboring GstSample wrapper indefinitely. +// Keep the writer's markers alive through the native leak check, not just EOS. +func TestWriterNewSampleTinyAllocations(t *testing.T) { + writer := &tinyAllocWriter{} + withNoGSTLeaks(t, func() { + require.NoError(t, writerNewSampleInner(context.Background(), 0, writer)) + }) + runtime.KeepAlive(writer) +} + +type tinyAllocWriter struct { + bytes.Buffer + markers []*uint64 +} + +func (w *tinyAllocWriter) Write(p []byte) (int, error) { + marker := new(uint64) + *marker = uint64(len(p)) + w.markers = append(w.markers, marker) + return w.Buffer.Write(p) +} + func writerNewSampleInner(ctx context.Context, i int, w io.Writer) error { ctx = log.WithLogValues(ctx, "func", "TestWriterNewSample") ctx, cancel := context.WithCancel(ctx)