[rust] Use file lock to protect concurrent accesses to cache

[bonigarcia]

User description

Description

The concurrent access to the cache folder by different SM processes might cause the following errors:

• Linux: error=26, Text file busy
• Windows: The process cannot access the file because it is being used by another process. (os error 32)

This PR implements a file locking mechanism to prevent this kind of problems.

Motivation and Context

Fix for #13511 and #13686.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Bug fix

---

Description

• Implemented file locking mechanism to prevent concurrent access issues to cache folder by different Selenium Manager processes
• Added Lock struct with acquire/release functionality using fs2 library
• Fixed file busy errors on Linux (error 26) and Windows (error 32)
• Refactored file moving operations into a reusable move_folder_content function
• Simplified decompression implementations using the new move function
• Added necessary dependencies (fs2, fs_extra) for file operations

---

Changes walkthrough 📝

	Relevant files
Bug fix	files.rs Implement file locking for concurrent cache access protection rust/src/files.rs Added file locking mechanism using Lock struct to prevent concurrent access to cache folder Implemented lock acquisition and release in uncompress function Refactored file moving logic into separate move_folder_content function Simplified uncompress_deb implementation using new move function +52/-20
Dependencies	Cargo.toml Add file system dependencies for locking mechanism              rust/Cargo.toml Added fs2 dependency for file locking functionality Added fs_extra dependency for directory operations +2/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The unwrap_or_default() calls in Lock implementation could silently ignore critical locking errors. Consider proper error handling or logging. Resource Cleanup Lock release in error cases may not be guaranteed. Consider using Drop trait implementation for Lock struct to ensure cleanup. Race Condition The lock file creation and exclusive locking are not atomic operations. Another process could acquire the lock between these steps.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	✅ Properly handle file lock acquisition errors instead of silently ignoring them Suggestion Impact: The commit completely removed the Lock struct and file locking functionality instead of improving error handling code diff: -use fs2::FileExt; use fs_extra::dir::{move_dir, CopyOptions}; use regex::Regex; use std::fs; @@ -55,7 +54,6 @@ const XZ: &str = "xz"; const SEVEN_ZIP_HEADER: &[u8; 6] = b"7z\xBC\xAF\x27\x1C"; const UNCOMPRESS_MACOS_ERR_MSG: &str = "{} files are only supported in macOS"; -const LOCK_FILE: &str = "sm.lock"; #[derive(Hash, Eq, PartialEq, Debug)] pub struct BrowserPath { @@ -69,35 +67,6 @@ os, channel: channel.to_string(), } - } -} - -pub struct Lock { - file: File, - path: PathBuf, -} - -impl Lock { - fn acquire(log: &Logger, target: &Path, single_file: Option<String>) -> Result<Self, Error> { - let lock_folder = if single_file.is_some() { - create_parent_path_if_not_exists(target)?; - target.parent().unwrap() - } else { - create_path_if_not_exists(target)?; - target - }; - let path = lock_folder.join(LOCK_FILE); - let file = File::create(&path)?; - - log.trace(format!("Using lock file at {}", path.display())); - file.lock_exclusive().unwrap_or_default(); - - Ok(Self { file, path }) - } - - fn release(&mut self) { - self.file.unlock().unwrap_or_default(); - fs::remove_file(&self.path).unwrap_or_default(); } Handle error cases in Lock::acquire() instead of using unwrap_or_default() which silently ignores lock failures. File locking is critical for preventing race conditions. rust/src/files.rs [93] -file.lock_exclusive().unwrap_or_default(); +file.lock_exclusive().map_err(|e| anyhow!("Failed to acquire file lock: {}", e))?; Apply this suggestion Suggestion importance[1-10]: 9 Why: Critical security improvement that prevents silent failures in file locking mechanism. Proper error handling is essential for maintaining data integrity and preventing race conditions.	9
	Properly handle file lock release errors instead of silently ignoring them Handle error cases in Lock::release() instead of using unwrap_or_default() which silently ignores unlock failures. Failed unlocks could leave the lock file in an inconsistent state. rust/src/files.rs [98-101] -fn release(&mut self) { - self.file.unlock().unwrap_or_default(); - fs::remove_file(&self.path).unwrap_or_default(); +fn release(&mut self) -> Result<(), Error> { + self.file.unlock().map_err(|e| anyhow!("Failed to release file lock: {}", e))?; + fs::remove_file(&self.path).map_err(|e| anyhow!("Failed to remove lock file: {}", e))?; + Ok(()) } Apply this suggestion Suggestion importance[1-10]: 8 Why: Important improvement to handle lock release failures which could leave the system in an inconsistent state. Converting to Result type allows proper error propagation.	8
	Propagate lock release errors instead of ignoring them Update the uncompress() function to handle the Result returned by lock.release() to ensure lock cleanup errors are properly propagated. rust/src/files.rs [197-198] -lock.release(); +lock.release()?; Ok(()) Apply this suggestion Suggestion importance[1-10]: 8 Why: Essential follow-up to the Lock::release() modification, ensuring lock cleanup errors are properly handled up the call stack.	8

[qodo-merge-pro]

CI Failure Feedback 🧐

(Checks updated until commit 1257d6d)

Action: Format / Check format script run

Failed stage: Run Bazel [❌]

Failure summary: The action failed during a file processing operation, likely related to adding license notices to source code files. The process exited with code 1, which indicates an error occurred while trying to use the Rust crate's logger and file system modules (logger::Logger and std::fs::File).

Relevant error logs: 1: ##[group]Operating System 2: Ubuntu ... 970: Package 'php-symfony-debug-bundle' is not installed, so not removed 971: Package 'php-symfony-dependency-injection' is not installed, so not removed 972: Package 'php-symfony-deprecation-contracts' is not installed, so not removed 973: Package 'php-symfony-discord-notifier' is not installed, so not removed 974: Package 'php-symfony-doctrine-bridge' is not installed, so not removed 975: Package 'php-symfony-doctrine-messenger' is not installed, so not removed 976: Package 'php-symfony-dom-crawler' is not installed, so not removed 977: Package 'php-symfony-dotenv' is not installed, so not removed 978: Package 'php-symfony-error-handler' is not installed, so not removed ... 1950: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/io/index.js 20ms (unchanged) 1951: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/io/zip.js 12ms (unchanged) 1952: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/jsdoc_conf.json 4ms (unchanged) 1953: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/lib/atoms/bidi-mutation-listener.js 4ms (unchanged) 1954: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/lib/atoms/make-atoms-module.js 4ms (unchanged) 1955: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/lib/by.js 19ms (unchanged) 1956: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/lib/capabilities.js 32ms (unchanged) 1957: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/lib/command.js 12ms (unchanged) 1958: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/lib/error.js 28ms (unchanged) ... 2031: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/http/util_test.js 14ms (unchanged) 2032: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/ie/options_test.js 20ms (unchanged) 2033: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/io/io_test.js 55ms (unchanged) 2034: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/io/zip_test.js 15ms (unchanged) 2035: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/lib/api_test.js 3ms (unchanged) 2036: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/lib/by_test.js 20ms (unchanged) 2037: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/lib/capabilities_test.js 25ms (unchanged) 2038: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/lib/credentials_test.js 16ms (unchanged) 2039: ../../../../../../../../../../work/selenium/selenium/javascript/node/selenium-webdriver/test/lib/error_test.js 24ms (unchanged) ... 2567: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/Firefox/FirefoxDriver.cs 2568: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/PrintOptions.cs 2569: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/BiDi/Communication/Command.cs 2570: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/Internal/Logging/Logger.cs 2571: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/BiDi/Modules/Module.cs 2572: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/TextHandlingTest.cs 2573: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/support/Events/WebDriverScriptEventArgs.cs 2574: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/support/GlobalSuppressions.cs 2575: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/WebDriverError.cs ... 2666: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/DevTools/v85/V85Domains.cs 2667: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/BiDi/Modules/Browser/CloseCommand.cs 2668: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/DevTools/Network.cs 2669: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/BiDi/EventArgs.cs 2670: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/BiDi/Input/DefaultMouseTest.cs 2671: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/BiDi/Modules/Input/SequentialSourceActions.cs 2672: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/BiDi/Modules/Network/Initiator.cs 2673: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/DevTools/ConsoleApiCalledEventArgs.cs 2674: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/ErrorResponse.cs ... 2902: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/support/Events/WebElementValueEventArgs.cs 2903: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/DevTools/DevToolsTestFixture.cs 2904: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs 2905: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/CookieTest.cs 2906: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/PageLoadingTest.cs 2907: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/NoSuchWindowException.cs 2908: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/PrintDocument.cs 2909: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/DriverProcessStartingEventArgs.cs 2910: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/ErrorsTest.cs ... 3009: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/Internal/Logging/ConsoleLogHandler.cs 3010: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/FrameSwitchingTest.cs 3011: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/support/Extensions/ExecuteJavaScriptTest.cs 3012: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/Environment/RemoteSeleniumServer.cs 3013: Adding notice to /home/runner/work/selenium/selenium/dotnet/test/common/BiDi/Network/NetworkEventsTest.cs 3014: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/XPathLookupException.cs 3015: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/BiDi/Modules/Network/Cookie.cs 3016: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/support/UI/SlowLoadableComponent{T}.cs 3017: Adding notice to /home/runner/work/selenium/selenium/dotnet/src/webdriver/BiDi/Modules/Network/FetchErrorEventArgs.cs ... 3164: +// software distributed under the License is distributed on an 3165: +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 3166: +// KIND, either express or implied. See the License for the 3167: +// specific language governing permissions and limitations 3168: +// under the License. 3169: + 3170: + 3171: use crate::logger::Logger; 3172: use anyhow::Error; 3173: use std::fs::File; 3174: ##[error]Process completed with exit code 1.

---

✨ CI feedback usage guide:

---

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

• Failed stage
• Failed test name
• Failure summary
• Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

• enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
• excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
• enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
• persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
• final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.