HLS/DASH: Fix dispose() to cleanup files after unpublish

[ppeeou]

HLS/DASH: Fix dispose() to cleanup files after unpublish

Summary

Fixes a bug where HLS/DASH files are not deleted after the configured hls_dispose/dash_dispose timeout.

Problem

When a stream is unpublished:

1. on_unpublish() is called and sets enabled_ = false
2. After the dispose timeout, cycle() calls dispose()
3. dispose() immediately returns due to if (!enabled_) check at line 2722 (HLS) and line 891 (DASH)
4. controller_->dispose() is never called
5. Files remain on disk indefinitely

Observed behavior:

• Stream stopped at 11:32:42
• dispose() called at 11:33:14 (after 30s timeout)
• Log shows "hls cycle to dispose hls" but no "gracefully dispose hls" message
• Files remain on disk

Root Cause

Commit 550760f introduced an early return in dispose() when !enabled_, which prevents file cleanup after on_unpublish() has already been called and set enabled_ to false.

Solution

Reorder the logic in dispose() to:

1. Check if dispose is enabled (hls_dispose/dash_dispose > 0) first
2. Call on_unpublish() only if enabled_ is still true (prevents duplicate calls)
3. Always call controller_->dispose() to cleanup files when dispose timeout occurs

This ensures files are properly cleaned up while still preventing duplicate on_unpublish() calls.

Changes Made

• trunk/src/app/srs_app_hls.cpp (lines 2718-2734): Reordered dispose() logic
• trunk/src/app/srs_app_dash.cpp (lines 887-902): Reordered dispose() logic
• trunk/doc/CHANGELOG.md: Added v7.0.137 entry

Testing Recommendation

To verify the fix:

1. Start RTMP stream to /live/test:

   ffmpeg -re -i test.mp4 -c copy -f flv rtmp://localhost:1935/live/test

2. Wait for HLS segments to be created:

   ls -la /path/to/hls/live/test/

3. Stop the stream (Ctrl+C)

4. Wait for hls_dispose timeout (default 120s, or 30s with your config):

   # Watch logs for "hls cycle to dispose hls" and "gracefully dispose hls"
   tail -f srs.log

5. Verify files are deleted:

   ls -la /path/to/hls/live/test/
   # Should be empty or directory removed

Expected results:

• Before fix: Files remain on disk
• After fix: Files are deleted, logs show "gracefully dispose hls"

Impact

• Risk: Low - minimal logic change, only reordering of checks
• Breaking changes: None
• Performance: No impact
• Compatibility: Fixes existing bug, improves expected behavior

Checklist

• Code follows project style
• Both HLS and DASH are fixed
• CHANGELOG updated
• Tested locally (recommended before merge)
• No breaking changes

Related Issues

• Regression introduced in: 550760f
• Related to: HLS: TS slices that have been generated should not be cleared when the server exits or restarts. #865 (hls_dispose feature)

[suzp1984]

I believe this PR make sense.

[suzp1984]

One more thing, Let the utest to cover this case.

e.g.

srs/trunk/src/utest/srs_utest_ai17.cpp

Lines 1381 to 1387 in 93c5d72

	// Test dispose() when enabled but dash_dispose is 0 - should call on_unpublish but not controller->dispose()
	dash->enabled_ = true;
	mock_config->dash_dispose_ = 0;
	dash->dispose();
	EXPECT_TRUE(mock_controller->on_unpublish_called_);
	EXPECT_FALSE(mock_controller->dispose_called_); // dash_dispose is 0, so controller->dispose() should not be called

To cover when dash/hls is enalbed_ == false, the dispose also called.

[ppeeou]

Thanks for the review feedback!

I've added the test case for DASH to cover enabled_ == false with dash_dispose > 0
The test now covers all 4 combinations for DASH dispose behavior and passes successfully.

However, I noticed there's no equivalent lifecycle/dispose test for HLS like DashTest.LifecycleInitializeDisposeCleanupDelay.

Question: Should I create a new HlsTest.LifecycleInitializeDisposeCleanupDelay test with the same coverage, or is the DASH test sufficient since both DASH and HLS have identical dispose logic?

[suzp1984]

Thanks for the review feedback!

I've added the test case for DASH to cover enabled_ == false with dash_dispose > 0 The test now covers all 4 combinations for DASH dispose behavior and passes successfully.

However, I noticed there's no equivalent lifecycle/dispose test for HLS like DashTest.LifecycleInitializeDisposeCleanupDelay.

Question: Should I create a new HlsTest.LifecycleInitializeDisposeCleanupDelay test with the same coverage, or is the DASH test sufficient since both DASH and HLS have identical dispose logic?

I think the independent HLS related utest is necessary, because they are independent codes.
Just cover what you commit is OK.

My suggest put the code in the srs_utest_ai17.cpp is OK.
The Test Case name maybe: HlsTest.LifecycleDisposeAfterDisabled.