[llvm-branch-commits] [llvm] release/23.x: [debuginfod] Abort cached file stream if request fails (#232038) (PR #232693)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Oct 16 04:32:40 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-support
Author: Nerixyz (Nerixyz)
<details>
<summary>Changes</summary>
Backports 6121ee7ed04d98e02b2f44c35242c51701b5d60c.
This fixes a crash (https://github.com/llvm/llvm-project/issues/184728).
---
Full diff: https://github.com/llvm/llvm-project/pull/232693.diff
8 Files Affected:
- (modified) llvm/include/llvm/HTTP/StreamedHTTPResponseHandler.h (+3)
- (modified) llvm/include/llvm/Support/Caching.h (+20-3)
- (modified) llvm/lib/Debuginfod/Debuginfod.cpp (+3-1)
- (modified) llvm/lib/HTTP/StreamedHTTPResponseHandler.cpp (+6)
- (modified) llvm/lib/Support/Caching.cpp (+6)
- (added) llvm/test/tools/llvm-debuginfod-find/Inputs/partial_response.py (+33)
- (added) llvm/test/tools/llvm-debuginfod-find/partial-response.test (+14)
- (modified) llvm/unittests/Support/Caching.cpp (+41)
``````````diff
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..1731d16a282cbd
--- /dev/null
+++ b/llvm/test/tools/llvm-debuginfod-find/partial-response.test
@@ -0,0 +1,14 @@
+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()));
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/232693
More information about the llvm-branch-commits
mailing list