[llvm-branch-commits] [llvm] release/23.x: [debuginfod] Abort cached file stream if request fails (#232038) (PR #232693)
Douglas Yung via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Oct 22 23:51:38 PDT 2026
https://github.com/dyung updated https://github.com/llvm/llvm-project/pull/232693
>From 1f43a9242e766c8df141dde41087b0df3bfeb7a3 Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev at outlook.de>
Date: Fri, 16 Oct 2026 12:48:26 +0200
Subject: [PATCH] [debuginfod] Abort cached file stream if request fails
(#232038)
When an HTTP request fails while it already received some data, the
cached file stream is never committed. Destroying it in that state
raises a fatal error, crashing the program (see #184728).
This PR adds an `abort` method that should be called in this situation
before destroying the stream. For the default implementation, only the
output stream is closed. For the `CacheStream` in `llvm::localCache`,
the temporary file is discarded as well.
The test is based on David's reproducer. Since WinHttp seems to discard
the content-length header, I switched to timing out the response
instead.
Fixes #184728. This is still a bug for the symstore locator in lldb, but
I'll fix that in a followup.
fix: disable test partial-response.test on system-windows
---
.../llvm/HTTP/StreamedHTTPResponseHandler.h | 3 ++
llvm/include/llvm/Support/Caching.h | 23 +++++++++--
llvm/lib/Debuginfod/Debuginfod.cpp | 4 +-
llvm/lib/HTTP/StreamedHTTPResponseHandler.cpp | 6 +++
llvm/lib/Support/Caching.cpp | 6 +++
.../Inputs/partial_response.py | 33 +++++++++++++++
.../partial-response.test | 15 +++++++
llvm/unittests/Support/Caching.cpp | 41 +++++++++++++++++++
8 files changed, 127 insertions(+), 4 deletions(-)
create mode 100644 llvm/test/tools/llvm-debuginfod-find/Inputs/partial_response.py
create mode 100644 llvm/test/tools/llvm-debuginfod-find/partial-response.test
diff --git a/llvm/include/llvm/HTTP/StreamedHTTPResponseHandler.h b/llvm/include/llvm/HTTP/StreamedHTTPResponseHandler.h
index ce9bf28655186d..1a4e5c83299048 100644
--- a/llvm/include/llvm/HTTP/StreamedHTTPResponseHandler.h
+++ b/llvm/include/llvm/HTTP/StreamedHTTPResponseHandler.h
@@ -39,6 +39,9 @@ class StreamedHTTPResponseHandler : public HTTPResponseHandler {
/// but before the StreamedHTTPResponseHandler object is destroyed.
Error commit();
+ /// Must be called if the result won't be committed.
+ Error abort();
+
virtual ~StreamedHTTPResponseHandler() = default;
Error handleBodyChunk(StringRef BodyChunk) override;
diff --git a/llvm/include/llvm/Support/Caching.h b/llvm/include/llvm/Support/Caching.h
index cebf071b0188f3..e571d758f23411 100644
--- a/llvm/include/llvm/Support/Caching.h
+++ b/llvm/include/llvm/Support/Caching.h
@@ -26,29 +26,46 @@ namespace llvm {
/// if a client needs to perform some action after the stream is written to,
/// that can be done by deriving from this class and overriding the destructor
/// or the commit() method.
+/// Before destruction of this class, either commit() or abort() must be called.
class CachedFileStream {
public:
CachedFileStream(std::unique_ptr<raw_pwrite_stream> OS,
std::string OSPath = "")
: OS(std::move(OS)), ObjectPathName(OSPath) {}
- /// Must be called exactly once after the writes to OS have been completed
+ /// Must be called after the writes to OS have been completed successfully
/// but before the CachedFileStream object is destroyed.
virtual Error commit() {
if (Committed)
return createStringError(make_error_code(std::errc::invalid_argument),
Twine("CacheStream already committed."));
+ if (Aborted)
+ return createStringError(make_error_code(std::errc::invalid_argument),
+ "CacheStream already aborted.");
Committed = true;
return Error::success();
}
+ /// Must be called if the file was not committed before its destroyed (for
+ /// example if some writes didn't complete).
+ virtual Error abort() {
+ if (Committed)
+ return createStringError(make_error_code(std::errc::invalid_argument),
+ "CacheStream already committed.");
+
+ OS.reset();
+ Aborted = true;
+ return Error::success();
+ }
+
bool Committed = false;
+ bool Aborted = false;
std::unique_ptr<raw_pwrite_stream> OS;
std::string ObjectPathName;
virtual ~CachedFileStream() {
- if (!Committed)
- report_fatal_error("CachedFileStream was not committed.\n");
+ if (!Committed && !Aborted)
+ report_fatal_error("CachedFileStream was not committed or aborted.\n");
}
};
diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp
index ac75ade516b2b7..1bb7812e4c7cbd 100644
--- a/llvm/lib/Debuginfod/Debuginfod.cpp
+++ b/llvm/lib/Debuginfod/Debuginfod.cpp
@@ -260,8 +260,10 @@ Expected<std::string> getCachedOrDownloadArtifact(
HTTPRequest Request(ArtifactUrl);
Request.Headers = getHeaders();
Error Err = Client.perform(Request, Handler);
- if (Err)
+ if (Err) {
+ consumeError(Handler.abort());
return std::move(Err);
+ }
if ((Err = Handler.commit()))
return std::move(Err);
diff --git a/llvm/lib/HTTP/StreamedHTTPResponseHandler.cpp b/llvm/lib/HTTP/StreamedHTTPResponseHandler.cpp
index 2ebe418cde0896..afb88d1c0cf012 100644
--- a/llvm/lib/HTTP/StreamedHTTPResponseHandler.cpp
+++ b/llvm/lib/HTTP/StreamedHTTPResponseHandler.cpp
@@ -31,4 +31,10 @@ Error StreamedHTTPResponseHandler::commit() {
return Error::success();
}
+Error StreamedHTTPResponseHandler::abort() {
+ if (FileStream)
+ return FileStream->abort();
+ return Error::success();
+}
+
} // namespace llvm
diff --git a/llvm/lib/Support/Caching.cpp b/llvm/lib/Support/Caching.cpp
index 40a5c44771b652..17d7edb4147a38 100644
--- a/llvm/lib/Support/Caching.cpp
+++ b/llvm/lib/Support/Caching.cpp
@@ -142,6 +142,12 @@ Expected<FileCache> llvm::localCache(const Twine &CacheNameRef,
AddBuffer(Task, ModuleName, std::move(*MBOrErr));
return Error::success();
}
+
+ Error abort() override {
+ Error E1 = CachedFileStream::abort();
+ Error E2 = TempFile.discard();
+ return joinErrors(std::move(E1), std::move(E2));
+ }
};
return [=](size_t Task, const Twine &ModuleName)
diff --git a/llvm/test/tools/llvm-debuginfod-find/Inputs/partial_response.py b/llvm/test/tools/llvm-debuginfod-find/Inputs/partial_response.py
new file mode 100644
index 00000000000000..2cef0b5cfc8048
--- /dev/null
+++ b/llvm/test/tools/llvm-debuginfod-find/Inputs/partial_response.py
@@ -0,0 +1,33 @@
+import http.server
+import os
+import subprocess
+import sys
+import threading
+
+
+class DelayingHandler(http.server.BaseHTTPRequestHandler):
+ can_respond = threading.Event()
+
+ def do_GET(self):
+ self.send_response(200)
+ self.send_header("Content-Type", "application/octet-stream")
+ self.send_header("Content-Length", "1000000")
+ self.end_headers()
+ for _ in range(10):
+ self.wfile.write(b"a" * 4096)
+ self.wfile.flush()
+ # Wait until the client has finished running.
+ self.can_respond.wait()
+
+
+httpd = http.server.HTTPServer(("localhost", 0), DelayingHandler)
+port = httpd.socket.getsockname()[1]
+
+try:
+ t = threading.Thread(target=httpd.serve_forever).start()
+ os.environ["DEBUGINFOD_URLS"] = f"http://localhost:{port}"
+ result = subprocess.run(sys.argv[1:], capture_output=True, timeout=600)
+ print(result.stderr.decode())
+finally:
+ DelayingHandler.can_respond.set()
+ httpd.shutdown()
diff --git a/llvm/test/tools/llvm-debuginfod-find/partial-response.test b/llvm/test/tools/llvm-debuginfod-find/partial-response.test
new file mode 100644
index 00000000000000..6a58c0152456f7
--- /dev/null
+++ b/llvm/test/tools/llvm-debuginfod-find/partial-response.test
@@ -0,0 +1,15 @@
+REQUIRES: curl && !system-windows, http-server
+
+
+RUN: rm -rf %t
+RUN: mkdir -p %t/debuginfod-cache
+
+RUN: env DEBUGINFOD_TIMEOUT=1 \
+RUN: env DEBUGINFOD_CACHE_PATH=%t/debuginfod-cache \
+RUN: env DEBUGINFOD_HEADERS_FILE=%S/Inputs/headers \
+RUN: %python %S/Inputs/partial_response.py llvm-debuginfod-find --debuginfo 0 | FileCheck %s
+RUN: env DEBUGINFOD_CACHE_PATH=%t/debuginfod-cache \
+RUN: %python -c 'import os; len(os.listdir(os.environ["DEBUGINFOD_CACHE_PATH"])) == 0 or exit(1)'
+
+CHECK: Build ID 00:
+CHECK-SAME: Timeout was reached
diff --git a/llvm/unittests/Support/Caching.cpp b/llvm/unittests/Support/Caching.cpp
index 41e6b85a5a1179..f0d892da7398cb 100644
--- a/llvm/unittests/Support/Caching.cpp
+++ b/llvm/unittests/Support/Caching.cpp
@@ -153,3 +153,44 @@ TEST(Caching, NoCommit) {
ASSERT_NO_ERROR(sys::fs::remove_directories(CacheDir.str()));
}
+
+TEST(Caching, Abort) {
+ SmallString<256> CacheDir;
+ sys::fs::createUniquePath("llvm_test_cache-%%%%%%", CacheDir, true);
+
+ sys::fs::remove_directories(CacheDir.str());
+
+ std::unique_ptr<MemoryBuffer> CachedBuffer;
+ auto AddBuffer = [&CachedBuffer](unsigned Task, const Twine &ModuleName,
+ std::unique_ptr<MemoryBuffer> M) {
+ CachedBuffer = std::move(M);
+ };
+ auto CacheOrErr =
+ localCache("LLVMTestCache", "LLVMTest", CacheDir, AddBuffer);
+ ASSERT_TRUE(bool(CacheOrErr));
+
+ FileCache &Cache = *CacheOrErr;
+
+ auto AddStreamOrErr = Cache(1, "foo", "");
+ ASSERT_TRUE(bool(AddStreamOrErr));
+
+ AddStreamFn &AddStream = *AddStreamOrErr;
+ ASSERT_TRUE(AddStream);
+
+ auto FileOrErr = AddStream(1, "");
+ ASSERT_TRUE(bool(FileOrErr));
+
+ CachedFileStream *CFS = FileOrErr->get();
+ (*CFS->OS).write(data, sizeof(data));
+ ASSERT_THAT_ERROR(CFS->abort(), Succeeded());
+
+ // Ensure the directory is still empty.
+ std::error_code EC;
+ sys::fs::directory_iterator DirIt(CacheDir.str(), EC);
+ ASSERT_NO_ERROR(EC);
+ ASSERT_EQ(DirIt, sys::fs::directory_iterator());
+
+ ASSERT_THAT_ERROR(CFS->commit(), Failed());
+
+ ASSERT_NO_ERROR(sys::fs::remove_directories(CacheDir.str()));
+}
More information about the llvm-branch-commits
mailing list