llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> The download path was derived only from the key and the PDB name, so two lookups (from different threads) potentially raced the same file path. Avoid this by creating using a unique suffix. Assisted-by: Claude --- Full diff: https://github.com/llvm/llvm-project/pull/214632.diff 1 Files Affected: - (modified) lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp (+26-12) ``````````diff diff --git a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp index eab334e0b8e7a..15a0adbb67670 100644 --- a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp +++ b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp @@ -261,22 +261,32 @@ RequestFileFromSymStoreServerHTTP(llvm::StringRef base_url, llvm::StringRef key, return {}; } - // Download into a temporary file. - llvm::SmallString<128> tmp_file; - constexpr bool erase_on_reboot = true; - path::system_temp_directory(erase_on_reboot, tmp_file); - path::append(tmp_file, llvm::formatv("lldb_symstore_{0}_{1}", key, pdb_name)); - - // Server has SymStore directory structure with forward slashes as separators. - std::string source_url = - llvm::formatv("{0}/{1}/{2}/{1}", base_url, pdb_name, key); - if (!llvm::HTTPClient::isAvailable()) { Debugger::ReportWarning( "HTTP client is not available for SymStore download"); return {}; } + // Download into a temporary file. The name must be unique: lookups for the + // same file can be in flight concurrently. + llvm::SmallString<128> tmp_model; + constexpr bool erase_on_reboot = true; + path::system_temp_directory(erase_on_reboot, tmp_model); + path::append(tmp_model, + llvm::formatv("lldb_symstore_{0}_{1}.%%%%%%", key, pdb_name)); + + llvm::SmallString<128> tmp_file; + if (std::error_code ec = fs::createUniqueFile(tmp_model, tmp_file)) { + Debugger::ReportWarning(llvm::formatv( + "failed to create a temporary file to download '{0}' into: {1}", + pdb_name, ec.message())); + return {}; + } + + // Server has SymStore directory structure with forward slashes as separators. + std::string source_url = + llvm::formatv("{0}/{1}/{2}/{1}", base_url, pdb_name, key); + llvm::HTTPClient client; client.setTimeout( std::chrono::seconds(GetGlobalPluginProperties().GetTimeout())); @@ -299,25 +309,29 @@ RequestFileFromSymStoreServerHTTP(llvm::StringRef base_url, llvm::StringRef key, Debugger::ReportWarning( llvm::formatv("failed to download from SymStore '{0}': {1}", source_url, llvm::toString(std::move(Err)))); + fs::remove(tmp_file); return {}; } if (llvm::Error Err = Handler.commit()) { Debugger::ReportWarning( llvm::formatv("failed to download from SymStore '{0}': {1}", source_url, llvm::toString(std::move(Err)))); + fs::remove(tmp_file); return {}; } unsigned responseCode = client.responseCode(); switch (responseCode) { - case 404: - return {}; // file not found case 200: return FileSpec(tmp_file.str()); // success + case 404: + fs::remove(tmp_file); // file not found + return {}; default: Debugger::ReportWarning(llvm::formatv( "failed to download from SymStore '{0}': response code {1}", source_url, responseCode)); + fs::remove(tmp_file); return {}; } } `````````` </details> https://github.com/llvm/llvm-project/pull/214632 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
