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

# 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 550760f2d 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`:
   ```bash
   ffmpeg -re -i test.mp4 -c copy -f flv rtmp://localhost:1935/live/test
   ```

2. Wait for HLS segments to be created:
   ```bash
   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):
   ```bash
# Watch logs for "hls cycle to dispose hls" and "gracefully dispose hls"
   tail -f srs.log
   ```

5. Verify files are deleted:
   ```bash
   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
- [x] Code follows project style
- [x] Both HLS and DASH are fixed
- [x] CHANGELOG updated
- [x] Tested locally (recommended before merge)
- [x] No breaking changes

## Related Issues
- Regression introduced in: 550760f2d
- Related to: #865 (hls_dispose feature)

---------

Co-authored-by: Jacob Su <suzp1984@gmail.com>

trunk/doc/CHANGELOG.md

@@ -7,6 +7,7 @@ The changelog for SRS.
 <a name="v7-changes"></a>
 
 ## SRS 7.0 Changelog
+* v7.0, 2025-12-31, Merge [#4618](https://github.com/ossrs/srs/pull/4618): HLS/DASH: Fix dispose() to cleanup files even after on_unpublish() sets enabled_ to false. v7.0.137 (#4618)
 * v7.0, 2025-12-07, Merge [#4602](https://github.com/ossrs/srs/pull/4602): HLS: Fix audio-only fMP4 playback skipping. v7.0.136 (#4602)
 * v7.0, 2025-12-06, Merge [#4604](https://github.com/ossrs/srs/pull/4604): DVR: Fix HEVC mp4 recording error. v7.0.135 (#4604)
 * v7.0, 2025-12-06, SRT: Fix peer_idle_timeout not applied to publishers and players. v7.0.134 (#4600)

trunk/src/app/srs_app_dash.cpp

@@ -886,13 +886,10 @@ SrsDash::~SrsDash()
 
 void SrsDash::dispose()
 {
-    // We disabled the reload, so DASH will not be enabled by reloading.
-    // As a result, if DASH is disabled, we don't need to dispose.
-    if (!enabled_) {
-        return;
-    }
-
+    // Call on_unpublish only if still enabled to avoid duplicate calls.
+    if (enabled_) {
         on_unpublish();
+    }
 
     // Ignore when dash_dispose disabled.
     srs_utime_t dash_dispose = config_->get_dash_dispose(req_->vhost_);
@@ -900,6 +897,7 @@ void SrsDash::dispose()
         return;
     }
 
+    // Always dispose files when dash_dispose timeout occurs, even if already unpublished.
     controller_->dispose();
 }
 

trunk/src/app/srs_app_hls.cpp

@@ -2717,13 +2717,10 @@ srs_error_t SrsHls::do_reload(int *reloading, int *reloaded, int *refreshed)
 // LCOV_EXCL_START
 void SrsHls::dispose()
 {
-    // We disabled the reload, so HLS will not be enabled by reloading.
-    // As a result, if HLS is disabled, we don't need to dispose.
-    if (!enabled_) {
-        return;
-    }
-
+    // Call on_unpublish only if still enabled to avoid duplicate calls.
+    if (enabled_) {
         on_unpublish();
+    }
 
     // Ignore when hls_dispose disabled.
     // @see https://github.com/ossrs/srs/issues/865
@@ -2732,6 +2729,7 @@ void SrsHls::dispose()
         return;
     }
 
+    // Always dispose files when hls_dispose timeout occurs, even if already unpublished.
     controller_->dispose();
 }
 // LCOV_EXCL_STOP

trunk/src/core/srs_core_version7.hpp

@@ -9,6 +9,6 @@
 
 #define VERSION_MAJOR 7
 #define VERSION_MINOR 0
-#define VERSION_REVISION 136
+#define VERSION_REVISION 137
 
 #endif

trunk/src/utest/srs_utest_ai17.cpp

@@ -1389,6 +1389,17 @@ VOID TEST(DashTest, LifecycleInitializeDisposeCleanupDelay)
     mock_controller->on_unpublish_called_ = false;
     mock_controller->dispose_called_ = false;
 
+    // Test dispose() when not enabled but dash_dispose is non-zero - should call controller->dispose() but not on_unpublish
+    dash->enabled_ = false;
+    mock_config->dash_dispose_ = 120 * SRS_UTIME_SECONDS;
+    dash->dispose();
+    EXPECT_FALSE(mock_controller->on_unpublish_called_); // enabled is false, so on_unpublish should not be called
+    EXPECT_TRUE(mock_controller->dispose_called_); // dash_dispose is non-zero, so controller->dispose() should be called
+
+    // Reset flags for next test
+    mock_controller->on_unpublish_called_ = false;
+    mock_controller->dispose_called_ = false;
+
     // Test dispose() when enabled and dash_dispose is non-zero - should call both on_unpublish and controller->dispose()
     dash->enabled_ = true;
     mock_config->dash_dispose_ = 120 * SRS_UTIME_SECONDS;
@@ -1522,6 +1533,74 @@ VOID TEST(DashTest, PublishLifecycleWithAudioVideo)
     srs_freep(mock_hub);
 }
 
+// Test SrsHls dispose behavior after disabled
+// This test covers dispose() logic when enabled_ is false but hls_dispose is configured
+VOID TEST(HlsTest, LifecycleDisposeAfterDisabled)
+{
+    // Create SrsHls object
+    SrsUniquePtr<SrsHls> hls(new SrsHls());
+
+    // Create mock dependencies
+    SrsUniquePtr<MockAppConfig> mock_config(new MockAppConfig());
+    SrsUniquePtr<MockSrsRequest> mock_req(new MockSrsRequest("test.vhost", "live", "livestream"));
+    MockHlsController *mock_controller = new MockHlsController();
+    MockOriginHub *mock_hub = new MockOriginHub();
+
+    // Inject mock config into SrsHls
+    hls->config_ = mock_config.get();
+
+    // Inject mock controller into SrsHls
+    srs_freep(hls->controller_);
+    hls->controller_ = mock_controller;
+
+    // Inject mock request
+    hls->req_ = mock_req.get();
+
+    // Test dispose() when not enabled and hls_dispose is 0 - should not call on_unpublish or controller->dispose()
+    mock_config->hls_dispose_ = 0;
+    hls->dispose();
+    EXPECT_EQ(0, mock_controller->on_unpublish_count_);
+    EXPECT_EQ(0, mock_controller->dispose_count_); // hls_dispose is 0, so controller->dispose() should not be called
+
+    // Test dispose() when enabled but hls_dispose is 0 - should call on_unpublish but not controller->dispose()
+    hls->enabled_ = true;
+    mock_config->hls_dispose_ = 0;
+    hls->dispose();
+    EXPECT_EQ(1, mock_controller->on_unpublish_count_);
+    EXPECT_EQ(0, mock_controller->dispose_count_); // hls_dispose is 0, so controller->dispose() should not be called
+
+    // Reset counts for next test
+    mock_controller->reset();
+
+    // Test dispose() when not enabled but hls_dispose is non-zero - should call controller->dispose() but not on_unpublish
+    hls->enabled_ = false;
+    mock_config->hls_dispose_ = 120 * SRS_UTIME_SECONDS;
+    hls->dispose();
+    EXPECT_EQ(0, mock_controller->on_unpublish_count_); // enabled is false, so on_unpublish should not be called
+    EXPECT_EQ(1, mock_controller->dispose_count_); // hls_dispose is non-zero, so controller->dispose() should be called
+
+    // Reset counts for next test
+    mock_controller->reset();
+
+    // Test dispose() when enabled and hls_dispose is non-zero - should call both on_unpublish and controller->dispose()
+    hls->enabled_ = true;
+    mock_config->hls_dispose_ = 120 * SRS_UTIME_SECONDS;
+    hls->dispose();
+
+    // Verify dispose() called on_unpublish when enabled
+    EXPECT_EQ(1, mock_controller->on_unpublish_count_);
+
+    // Verify dispose() called controller->dispose() when hls_dispose is enabled
+    EXPECT_EQ(1, mock_controller->dispose_count_);
+
+    // Clean up - set to NULL to avoid double-free
+    hls->config_ = NULL;
+    hls->controller_ = NULL;
+    hls->req_ = NULL;
+    srs_freep(mock_controller);
+    srs_freep(mock_hub);
+}
+
 // Test SrsFragmentedMp4 delegation to fragment_ member
 // This test covers the major use scenario for SrsFragmentedMp4 ISrsFragment interface delegation
 VOID TEST(FragmentedMp4Test, FragmentDelegation)

trunk/src/utest/srs_utest_ai17.hpp

@@ -15,6 +15,7 @@
 #include <srs_app_dash.hpp>
 #include <srs_app_factory.hpp>
 #include <srs_app_fragment.hpp>
+#include <srs_app_hls.hpp>
 #include <srs_app_rtc_api.hpp>
 #include <srs_app_server.hpp>
 #include <srs_app_statistic.hpp>

trunk/src/utest/srs_utest_manual_mock.hpp

@@ -311,6 +311,7 @@ public:
     bool rtc_to_rtmp_;
     srs_utime_t dash_dispose_;
     bool dash_enabled_;
+    srs_utime_t hls_dispose_;
     bool api_as_candidates_;
     bool resolve_api_domain_;
     bool keep_api_domain_;
@@ -343,6 +344,7 @@ public:
         rtc_to_rtmp_ = false;
         dash_dispose_ = 0;
         dash_enabled_ = false;
+        hls_dispose_ = 0;
         api_as_candidates_ = true;
         resolve_api_domain_ = true;
         keep_api_domain_ = false;
@@ -562,7 +564,7 @@ public:
     virtual srs_utime_t get_hls_window(std::string vhost) { return 60 * SRS_UTIME_SECONDS; }
     virtual std::string get_hls_on_error(std::string vhost) { return "continue"; }
     virtual bool get_hls_cleanup(std::string vhost) { return true; }
-    virtual srs_utime_t get_hls_dispose(std::string vhost) { return 120 * SRS_UTIME_SECONDS; }
+    virtual srs_utime_t get_hls_dispose(std::string vhost) { return hls_dispose_; }
     virtual bool get_hls_wait_keyframe(std::string vhost) { return true; }
     virtual bool get_hls_keys(std::string vhost) { return false; }
     virtual int get_hls_fragments_per_key(std::string vhost) { return 5; }