OBS source: os_sleep_ms teardown is a timing-based use-after-free race

[kixelated]

In cpp/obs/src/moq-source.cpp, moq_source_destroy frees ctx after a fixed os_sleep_ms(100) that is meant to let in-flight async MoQ callbacks drain (they check shutting_down and exit early). This is a timing-based workaround, not a synchronization guarantee — if a callback is mid-execution when shutting_down is set and runs longer than 100ms, it touches ctx after it is freed. The code documents this limitation itself:

moq/cpp/obs/src/moq-source.cpp

Lines 170 to 196 in d5eef88

	moq_source_disconnect_locked(ctx);
	pthread_mutex_unlock(&ctx->mutex);
	// Give MoQ callbacks time to drain - they check shutting_down and exit early.
	// This prevents use-after-free when async callbacks fire after ctx is freed.
	//
	// LIMITATION: This 100ms sleep is a timing-based workaround, not a synchronization
	// guarantee. If a callback is mid-execution when shutting_down is set AND takes
	// longer than 100ms to complete (after the mutex unlock), there is still a
	// potential race condition. In practice, our callbacks are fast (< 1ms typically)
	// and this delay provides sufficient margin. However, a more robust solution
	// would use reference counting:
	// - Increment refcount when entering a callback
	// - Decrement when exiting
	// - Wait for refcount to reach zero before freeing ctx
	// This could be implemented using std::shared_ptr or a manual atomic refcount
	// with a condition variable for waiting.
	os_sleep_ms(100);
	bfree(ctx->url);
	bfree(ctx->broadcast);
	// Note: frame_buffer is already freed by moq_source_disconnect_locked
	pthread_mutex_destroy(&ctx->mutex);
	bfree(ctx);
	}

Why 100ms is not safe

The decode callback (moq_source_decode_frame) runs FFmpeg avcodec_send_packet / avcodec_receive_frame, which can exceed 100ms on a video frame (large/keyframe, slow HW, contention). The "callbacks are < 1ms typically" assumption does not hold for the decode path, so the UAF is reachable in practice on shutdown/scene-switch.

Proper fix

Replace the sleep with real quiescence:

• An atomic in-flight-callback counter: increment on callback entry, decrement on exit.
• Before freeing ctx, wait for the counter to reach zero (condition variable), rather than sleeping a fixed interval.

(A std::shared_ptr-based ownership model for ctx is a viable alternative.)

Related

A sibling timing-based delay exists in the reconnect path (os_sleep_ms(50)), same pattern, lower severity — worth addressing together:

moq/cpp/obs/src/moq-source.cpp

Lines 454 to 459 in d5eef88

	// Blank video while reconnecting to avoid showing stale frames
	moq_source_blank_video(ctx);
	// Small delay to allow MoQ library to fully clean up previous connection
	os_sleep_ms(50);

Context

Pre-existing in the vendored OBS plugin source; surfaced during the review of #1787 and deferred there rather than patched blind (it needs runtime testing). cpp/obs is the canonical home for the plugin now.

(Written by Claude)

[coderabbitai]

🔗 Related PRs

#1782 - fix(ci): unbreak the self-hosted A1 runner (libmoq clippy + ffmpeg bindgen headers) [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[kixelated]

moved back to the OBS repo