Cvs 194742 progress bar fix

src/pull_module/curl_downloader.cpp

@@ -15,10 +15,12 @@
 //*****************************************************************************
 #include "curl_downloader.hpp"
 
+#include <cstdlib>
 #include <ctime>
 #include <filesystem>
 #include <iostream>
 #include <memory>
+#include <mutex>
 #include <string>
 
 #include <curl/curl.h>
@@ -46,13 +48,45 @@ static void print_download_speed_info(size_t received_size, size_t elapsed_time)
     printf(" [%.2f %s/s] ", rate, sizeUnits[rate_unit_idx]);
 }
 
+int computeProgressBarCells(size_t count, size_t max, int barWidth) {
+    if (max == 0 || barWidth <= 0) {
+        return 0;
+    }
+    const double ratio = static_cast<double>(count) / static_cast<double>(max);
+    // Written as a positive test so a NaN ratio also lands here rather than falling through.
+    if (!(ratio > 0.0)) {
+        return 0;
+    }
+    if (ratio >= 1.0) {
+        return barWidth;
+    }
+    return static_cast<int>(ratio * barWidth);
+}
+
 static void print_progress(size_t count, size_t max, bool first_run, size_t elapsed_time) {
+    // A response with no Content-Length reports dltotal == 0, so there is no ratio to show;
+    // report the running byte count instead. Dividing by max here yielded an infinite ratio
+    // whose conversion to int is undefined - in practice INT_MIN, which drove the padding
+    // loop below through roughly 2.1 billion putchar calls on every progress tick.
+    if (max == 0) {
+        double received = (double)count;
+        size_t receivedUnitId = 0;
+        while (received > 1000 && sizeUnits[receivedUnitId + 1]) {
+            received /= 1000.0;
+            receivedUnitId++;
+        }
+        printf("\rProgress: %.2f %s downloaded, total size unknown", received, sizeUnits[receivedUnitId]);
+        print_download_speed_info(count, elapsed_time);
+        fflush(stdout);
+        return;
+    }
+
     float progress = (float)count / max;
     if (!first_run && progress < 0.01 && count > 0)
         return;
 
     const int bar_width = 50;
-    int bar_length = progress * bar_width;
+    const int bar_length = computeProgressBarCells(count, max, bar_width);
 
     printf("\rProgress: [");
     int i;
@@ -117,6 +151,25 @@ static size_t file_write_callback(void* buffer, size_t size, size_t nmemb, void*
         }                                                                                                \
     } while (0)
 
+// libcurl requires curl_global_init/curl_global_cleanup to run exactly once per process;
+// calling curl_global_cleanup() after every download tears down global TLS/engine state
+// still needed elsewhere (other in-flight curl users), which segfaults on next use.
+static Status ensureCurlGlobalInit() {
+    static std::once_flag initFlag;
+    static CURLcode initResult = CURLE_OK;
+    std::call_once(initFlag, []() {
+        initResult = curl_global_init(CURL_GLOBAL_DEFAULT);
+        if (initResult == CURLE_OK) {
+            std::atexit([]() { curl_global_cleanup(); });
+        }
+    });
+    if (initResult != CURLE_OK) {
+        SPDLOG_ERROR("curl error: {}. Error code: {}", curl_easy_strerror(initResult), (int)initResult);
+        return StatusCode::INTERNAL_ERROR;
+    }
+    return StatusCode::OK;
+}
+
 struct ProgressData {
     time_t started_download;
     time_t last_print_time;
@@ -159,9 +212,10 @@ Status downloadFileWithCurl(const std::string& url, const std::string& filePath,
     std::string agentString = std::string(PROJECT_NAME) + "/" + std::string(PROJECT_VERSION);
 
     CURL* curl = nullptr;
-    CHECK_CURL_CALL(curl_global_init(CURL_GLOBAL_DEFAULT));
-    auto globalCurlGuard = std::unique_ptr<void, void (*)(void*)>(
-        nullptr, [](void*) { curl_global_cleanup(); });
+    auto initStatus = ensureCurlGlobalInit();
+    if (!initStatus.ok()) {
+        return initStatus;
+    }
     curl = curl_easy_init();
     if (!curl) {
         SPDLOG_ERROR("Failed to initialize cURL.");
@@ -211,9 +265,10 @@ Status fetchUrlToString(const std::string& url, const std::string& authToken, st
     std::string agentString = std::string(PROJECT_NAME) + "/" + std::string(PROJECT_VERSION);
 
     CURL* curl = nullptr;
-    CHECK_CURL_CALL(curl_global_init(CURL_GLOBAL_DEFAULT));
-    auto globalCurlGuard = std::unique_ptr<void, void (*)(void*)>(
-        nullptr, [](void*) { curl_global_cleanup(); });
+    auto initStatus = ensureCurlGlobalInit();
+    if (!initStatus.ok()) {
+        return initStatus;
+    }
     curl = curl_easy_init();
     if (!curl) {
         SPDLOG_ERROR("Failed to initialize cURL.");

src/pull_module/curl_downloader.hpp

@@ -14,6 +14,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 //*****************************************************************************
+#include <cstddef>
 #include <string>
 
 namespace ovms {
@@ -23,4 +24,9 @@ Status downloadFileWithCurl(const std::string& url, const std::string& filePath)
 Status downloadFileWithCurl(const std::string& url, const std::string& filePath, const std::string& authTokenHF);
 Status fetchUrlToString(const std::string& url, const std::string& authToken, std::string& responseBody);
 
+// Number of filled cells in a barWidth-wide progress bar for count out of max bytes,
+// clamped to [0, barWidth]. max == 0 means the server sent no Content-Length, so there is
+// no ratio to render and the result is 0. Declared here so the arithmetic can be unit tested.
+int computeProgressBarCells(size_t count, size_t max, int barWidth);
+
 }  // namespace ovms

src/test/pull_hf_model_test.cpp

@@ -13,11 +13,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 //*****************************************************************************
+#include <algorithm>
 #include <array>
 #include <chrono>
 #include <cstdlib>
 #include <fstream>
+#include <future>
 #include <iomanip>
+#include <limits>
 #include <memory>
 #include <openssl/sha.h>
 #include <mutex>
@@ -47,6 +50,7 @@
 #include "src/test/test_file_utils.hpp"
 #include "src/test/test_with_temp_dir.hpp"
 #include "src/filesystem/filesystem.hpp"
+#include "src/pull_module/curl_downloader.hpp"
 #include "src/pull_module/hf_pull_model_module.hpp"
 #include "src/pull_module/libgit2.hpp"
 #include "src/pull_module/optimum_export.hpp"
@@ -378,6 +382,75 @@ ::testing::AssertionResult interruptPosixWorkerAndExpectGracefulExit(pid_t child
 
 }  // namespace
 
+// A response without Content-Length makes libcurl report dltotal == 0. The progress bar must
+// not divide by it: the ratio becomes infinite and converting that to int is undefined, which
+// in practice produced INT_MIN and a ~2.1 billion iteration padding loop.
+TEST(CurlDownloaderProgressTest, UnknownTotalYieldsNoFilledCells) {
+    EXPECT_EQ(ovms::computeProgressBarCells(0, 0, 50), 0);
+    EXPECT_EQ(ovms::computeProgressBarCells(1024, 0, 50), 0);
+    EXPECT_EQ(ovms::computeProgressBarCells(std::numeric_limits<size_t>::max(), 0, 50), 0);
+}
+
+TEST(CurlDownloaderProgressTest, FilledCellsTrackRatio) {
+    EXPECT_EQ(ovms::computeProgressBarCells(0, 100, 50), 0);
+    EXPECT_EQ(ovms::computeProgressBarCells(50, 100, 50), 25);
+    EXPECT_EQ(ovms::computeProgressBarCells(100, 100, 50), 50);
+}
+
+// Some servers report more bytes transferred than announced; the bar must stay within its width
+// so the padding loop below it always runs a sane number of times.
+TEST(CurlDownloaderProgressTest, FilledCellsClampToBarWidth) {
+    EXPECT_EQ(ovms::computeProgressBarCells(200, 100, 50), 50);
+    EXPECT_EQ(ovms::computeProgressBarCells(100, 100, 0), 0);
+    EXPECT_EQ(ovms::computeProgressBarCells(100, 100, -1), 0);
+}
+
+// Regression test for the call-site, not just the extracted helper: a real chunked-transfer
+// response (no Content-Length) drives libcurl's progress callback with dltotal == 0 on every
+// tick. Before the fix this hung the download in a ~2.1 billion iteration padding loop; here
+// we bound the wait so a reintroduced regression fails instead of hanging the test suite.
+TEST_F(TestWithTempDir, ChunkedTransferWithoutContentLengthDoesNotHang) {
+    const std::string body(64 * 1024, 'x');
+    httplib::Server server;
+    server.Get("/chunked", [&body](const httplib::Request&, httplib::Response& res) {
+        res.set_chunked_content_provider("application/octet-stream",
+            [&body](size_t offset, httplib::DataSink& sink) {
+                if (offset >= body.size()) {
+                    sink.done();
+                    return true;
+                }
+                const size_t chunkSize = std::min<size_t>(4096, body.size() - offset);
+                sink.write(body.data() + offset, chunkSize);
+                return true;
+            });
+    });
+    const int port = server.bind_to_any_port("127.0.0.1");
+    ASSERT_GT(port, 0);
+    std::thread serverThread([&server]() {
+        server.listen_after_bind();
+    });
+    server.wait_until_ready();
+
+    const std::string url = "http://127.0.0.1:" + std::to_string(port) + "/chunked";
+    const std::string outputPath = directoryPath + "/downloaded.bin";
+
+    auto downloadFuture = std::async(std::launch::async, [&url, &outputPath]() {
+        return ovms::downloadFileWithCurl(url, outputPath);
+    });
+
+    ASSERT_EQ(downloadFuture.wait_for(std::chrono::seconds(10)), std::future_status::ready)
+        << "downloadFileWithCurl did not return in time for a chunked, Content-Length-less response";
+    EXPECT_EQ(downloadFuture.get(), ovms::StatusCode::OK);
+
+    server.stop();
+    serverThread.join();
+
+    std::ifstream downloadedFile(outputPath, std::ios::binary);
+    std::ostringstream downloadedContent;
+    downloadedContent << downloadedFile.rdbuf();
+    EXPECT_EQ(downloadedContent.str(), body);
+}
+
 // RAII helper class for managing log file lifecycle.
 // Creates a log file path and automatically removes it on destruction.
 class LogFileGuard {
@@ -2453,7 +2526,8 @@ TEST_F(HfPullModelModuleLoraTest, ResolveHfLoraFilenames) {
     ovms::ImageGenerationGraphSettingsImpl graphSettings;
     ovms::LoraAdapterSettings adapter;
     adapter.alias = "pokemon";
-    adapter.sourceLora = "juliensimon/sd-pokemon-lora";
+    // juliensimon/sd-pokemon-lora was removed upstream; replaced with a repo verified to still exist.
+    adapter.sourceLora = "MohamedAhmedAE/stable-diffusion-v1-5_lora_finetuning";
     adapter.sourceType = ovms::LoraSourceType::HF_REPO;
     graphSettings.loraAdapters.push_back(adapter);
     settings.graphSettings = graphSettings;
@@ -2480,7 +2554,8 @@ TEST_F(HfPullModelModuleLoraTest, PullLoraAdaptersFromHfRepo) {
     ovms::ImageGenerationGraphSettingsImpl graphSettings;
     ovms::LoraAdapterSettings adapter;
     adapter.alias = "pokemon";
-    adapter.sourceLora = "juliensimon/sd-pokemon-lora";
+    // juliensimon/sd-pokemon-lora was removed upstream; replaced with a repo verified to still exist.
+    adapter.sourceLora = "MohamedAhmedAE/stable-diffusion-v1-5_lora_finetuning";
     adapter.safetensorsFile = "pytorch_lora_weights.safetensors";  // explicit filename — skips HF API resolve
     adapter.sourceType = ovms::LoraSourceType::HF_REPO;
     graphSettings.loraAdapters.push_back(adapter);
@@ -2489,7 +2564,7 @@ TEST_F(HfPullModelModuleLoraTest, PullLoraAdaptersFromHfRepo) {
     auto status = module.testPullLoraAdapters(this->directoryPath);
     ASSERT_TRUE(status.ok()) << status.string();
 
-    auto loraFilePath = ovms::FileSystem::joinPath({this->directoryPath, "loras", "juliensimon/sd-pokemon-lora", "pytorch_lora_weights.safetensors"});
+    auto loraFilePath = ovms::FileSystem::joinPath({this->directoryPath, "loras", "MohamedAhmedAE/stable-diffusion-v1-5_lora_finetuning", "pytorch_lora_weights.safetensors"});
     ASSERT_TRUE(std::filesystem::exists(loraFilePath)) << loraFilePath;
     EXPECT_GT(std::filesystem::file_size(loraFilePath), 0);
 }
@@ -2539,7 +2614,8 @@ TEST_F(HfDownloaderPullHfModel, DownloadImageGenModelWithLoRA) {
     std::string modelName = "OpenVINO/stable-diffusion-v1-5-int8-ov";
     std::string downloadPath = ovms::FileSystem::joinPath({this->directoryPath, "repository"});
     std::string task = "image_generation";
-    std::string sourceLoras = "pokemon=juliensimon/sd-pokemon-lora@pytorch_lora_weights.safetensors";
+    // juliensimon/sd-pokemon-lora was removed upstream; replaced with a repo verified to still exist.
+    std::string sourceLoras = "pokemon=MohamedAhmedAE/stable-diffusion-v1-5_lora_finetuning@pytorch_lora_weights.safetensors";
     ::SetUpServerForDownloadWithLoras(this->t, this->server, modelName, downloadPath, task, sourceLoras);
 
     std::string basePath = ovms::FileSystem::joinPath({downloadPath, "OpenVINO", "stable-diffusion-v1-5-int8-ov"});
@@ -2550,7 +2626,7 @@ TEST_F(HfDownloaderPullHfModel, DownloadImageGenModelWithLoRA) {
     ASSERT_TRUE(std::filesystem::exists(graphPath)) << graphPath;
 
     // Verify LoRA adapter was downloaded
-    std::string loraDir = ovms::FileSystem::joinPath({basePath, "loras", "juliensimon", "sd-pokemon-lora"});
+    std::string loraDir = ovms::FileSystem::joinPath({basePath, "loras", "MohamedAhmedAE", "stable-diffusion-v1-5_lora_finetuning"});
     auto loraFiles = searchFilesRecursively(loraDir, {"pytorch_lora_weights.safetensors"});
     ASSERT_FALSE(loraFiles.empty()) << "LoRA .safetensors not found in: " << loraDir;
 

third_party/libgit2/lfs.patch

@@ -427,7 +427,7 @@ new file mode 100644
 index 000000000..18490e5ad
 --- /dev/null
 +++ b/src/libgit2/lfs_filter.c
-@@ -0,0 +1,2014 @@
+@@ -0,0 +1,2015 @@
 +/*
 +/ Copyright 2025 Intel Corporation
 +/
@@ -1565,7 +1565,8 @@ index 000000000..18490e5ad
 +		return;
 +
 +	bar_width = 50;
-+	bar_length = progress * bar_width;
++	/* Clamp: a server reporting more bytes than dltotal must not overflow bar_width. */
++	bar_length = (progress >= 1.0) ? bar_width : (int)(progress * bar_width);
 +
 +	printf("\rProgress: [");
 +	for (i = 0; i < bar_length; ++i) {