Skip to content

Commit 393bc23

Browse files
authored
Revert "VPLAY-11880:The manifest download keeps failing, eventually causing A…" (#738)
This reverts commit 41c0251.
1 parent 37e33f4 commit 393bc23

File tree

5 files changed

+2
-78
lines changed

5 files changed

+2
-78
lines changed

AampMPDDownloader.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ void AampMPDDownloader::Release()
279279
mMPDNotifierCondVar.notify_all();
280280

281281
}
282-
//Disable downloads before joining the threads,which will exit the download loops gracefully
282+
283283
mDownloader1.Release();
284284
mDownloader2.Release();
285285

@@ -288,9 +288,6 @@ void AampMPDDownloader::Release()
288288

289289
if(mDownloaderThread_t2.joinable())
290290
mDownloaderThread_t2.join();
291-
// Clear the headers only after the graceful exit of download threads.
292-
mDownloader1.CleanupCurlHeaderResources();
293-
mDownloader2.CleanupCurlHeaderResources();
294291

295292
if(mManifestUpdateCb != NULL)
296293
{

downloader/AampCurlDownloader.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -372,11 +372,6 @@ void AampCurlDownloader::Release()
372372
{
373373
std::lock_guard<std::mutex> lock(mCurlMutex);
374374
mDownloadActive = false;
375-
}
376-
377-
void AampCurlDownloader::CleanupCurlHeaderResources()
378-
{
379-
std::lock_guard<std::mutex> lock(mCurlMutex);
380375
mDownloadUpdatedTime = 0 ;
381376
mDownloadStartTime = 0;
382377
mWriteCallbackBufferSize = 0;

downloader/AampCurlDownloader.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,10 +228,6 @@ class AampCurlDownloader
228228
* @param[in] dnldCfg - configuration for download
229229
*/
230230
void Release();
231-
/**
232-
* @brief CleanupCurlHeaderResources - function to cleanup headers after thread join
233-
*/
234-
void CleanupCurlHeaderResources();
235231
void Clear();
236232
/**
237233
* @brief Download - function to start download

test/utests/fakes/FakeAampCurlDownloader.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,6 @@ void AampCurlDownloader::Release()
9898
{
9999
}
100100

101-
void AampCurlDownloader::CleanupCurlHeaderResources()
102-
{
103-
}
104-
105101

106102
void AampCurlDownloader::Clear()
107103
{

test/utests/tests/AampCurlDownloader/FunctionalTests.cpp

Lines changed: 1 addition & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -466,64 +466,4 @@ TEST_F(FunctionalTests, AampCurlDownloader_Retry_502)
466466
EXPECT_EQ(408, respData->iHttpRetValue);
467467
respData->show();
468468
respData->clear();
469-
}
470-
471-
/**
472-
* @brief Test case to verify calling order: Release() before CleanupCurlHeaderResources()
473-
*
474-
* This test demonstrates the recommended sequence where Release() is called first to stop
475-
* downloads (sets mDownloadActive=false), followed by CleanupCurlHeaderResources() to free
476-
* curl header resources. This ordering prevents potential race conditions where header
477-
* resources could be freed while curl callbacks are still executing.
478-
*
479-
*/
480-
TEST_F(FunctionalTests, Release_BeforeCleanupCurlHeaderResources_PreventRaceCondition) {
481-
DownloadConfigPtr config = std::make_shared<DownloadConfig>();
482-
483-
EXPECT_CALL(*g_mockCurl, curl_easy_init()).WillOnce(Return(mCurlEasyHandle));
484-
EXPECT_CALL(*g_mockCurl, curl_easy_cleanup(mCurlEasyHandle));
485-
EXPECT_CALL(*g_mockCurl, curl_easy_setopt_ptr(mCurlEasyHandle, CURLOPT_PROGRESSDATA, mAampCurlDownloader))
486-
.WillOnce(Return(CURLE_OK));
487-
EXPECT_CALL(*g_mockCurl, curl_easy_setopt_func_xferinfo(mCurlEasyHandle, CURLOPT_XFERINFOFUNCTION, NotNull()))
488-
.WillOnce(DoAll(SaveArgPointee<2>(&mCurlProgressCallback), Return(CURLE_OK)));
489-
EXPECT_CALL(*g_mockCurl, curl_easy_setopt_ptr(mCurlEasyHandle, CURLOPT_WRITEDATA, mAampCurlDownloader))
490-
.WillOnce(Return(CURLE_OK));
491-
EXPECT_CALL(*g_mockCurl, curl_easy_setopt_func_write(mCurlEasyHandle, CURLOPT_WRITEFUNCTION, NotNull()))
492-
.WillOnce(DoAll(SaveArgPointee<2>(&mCurlWriteFunc), Return(CURLE_OK)));
493-
494-
mAampCurlDownloader->Initialize(config);
495-
496-
std::atomic<bool> releaseCompleted(false);
497-
std::atomic<bool> headerCleanupStarted(false);
498-
std::atomic<bool> raceConditionDetected(false);
499-
500-
// Thread 1: Simulates download activity
501-
std::thread downloadThread([&]() {
502-
for (int i = 0; i < 100 && !releaseCompleted.load(); ++i) {
503-
if (headerCleanupStarted.load() && !releaseCompleted.load()) {
504-
raceConditionDetected.store(true);
505-
}
506-
std::this_thread::sleep_for(std::chrono::milliseconds(1));
507-
}
508-
});
509-
510-
// Allow download thread to start
511-
std::this_thread::sleep_for(std::chrono::milliseconds(10));
512-
513-
// Step 1: Call Release() first to stop downloads
514-
mAampCurlDownloader->Release();
515-
releaseCompleted.store(true);
516-
517-
// Step 2: Call CleanupCurlHeaderResources() after downloads stopped
518-
headerCleanupStarted.store(true);
519-
mAampCurlDownloader->CleanupCurlHeaderResources();
520-
521-
downloadThread.join();
522-
523-
// Verify no race condition detected
524-
EXPECT_FALSE(raceConditionDetected.load())
525-
<< "Race condition detected: CleanupCurlHeaderResources() called before Release() completed";
526-
527-
// Verify download is stopped
528-
EXPECT_FALSE(mAampCurlDownloader->IsDownloadActive());
529-
}
469+
}

0 commit comments

Comments
 (0)