[java] Synchronize method to get Selenium Manager binary (fix #11620)#11640
[java] Synchronize method to get Selenium Manager binary (fix #11620)#11640
Conversation
|
@bonigarcia - I think there's a larger problem here.
Assumptions (Only this will kick in the SeleniumManager into action)
I think the fix should be to do something like below (I have changed public enum SeleniumManager {
manager;
private static final Logger LOG = Logger.getLogger(SeleniumManager.class.getName());
private static final String SELENIUM_MANAGER = "selenium-manager";
private static final String EXE = ".exe";
private static final String INFO = "INFO\t";
//We cache the binary path on a per driver flavor basis.
private final Map<String, File> binaries = new ConcurrentHashMap<>();
SeleniumManager() {
Runtime.getRuntime().addShutdownHook(new Thread(() -> binaries.values()
.stream()
.filter(Objects::nonNull)
.filter(File::exists)
.forEach(SeleniumManager::deleteQuietly)));
}
private static void deleteQuietly(File binary) {
try {
Files.delete(binary.toPath());
} catch (IOException e) {
LOG.warning(String.format("%s deleting temporal file: %s",
e.getClass().getSimpleName(), e.getMessage()));
}
}
public SeleniumManager getInstance() {
return manager;
}
private File getBinary(String driverName) {
return this.binaries.computeIfAbsent(driverName, flavor -> {
try {
Platform current = Platform.getCurrent();
String folder = "linux";
String extension = "";
if (current.is(WINDOWS)) {
extension = EXE;
folder = "windows";
} else if (current.is(MAC)) {
folder = "macos";
}
String binaryPath = String.format("%s/%s%s", folder, SELENIUM_MANAGER, extension);
try (InputStream inputStream = this.getClass().getResourceAsStream(binaryPath)) {
Path tmpPath = Files.createTempDirectory(SELENIUM_MANAGER + System.nanoTime());
File tmpFolder = tmpPath.toFile();
tmpFolder.deleteOnExit();
File binary = new File(tmpFolder, SELENIUM_MANAGER + extension);
Files.copy(inputStream, binary.toPath(), REPLACE_EXISTING);
binary.setExecutable(true);
return binary;
}
} catch (Exception e) {
throw new WebDriverException("Unable to obtain Selenium Manager", e);
}
});
}
public String getDriverPath(String driverName) {
if (!ImmutableList.of("geckodriver", "chromedriver", "msedgedriver", "IEDriverServer").contains(driverName)) {
throw new WebDriverException("Unable to locate driver with name: " + driverName);
}
String driverPath = null;
File binaryFile = getBinary(driverName);
if (binaryFile != null) {
driverPath = runCommand(binaryFile.getAbsolutePath(),
"--driver", driverName.replaceAll(EXE, ""));
}
return driverPath;
}
} |
That is wrong. That binary path is the Selenium Manager binary, not the driver. The Selenium Manager binary is used to manage (i.e., download, cache, etc.) the drivers (geckodriver, chromedriver, etc.). |
Ah! Yeah now I realise my blunder. The |
|
Is the issue getting the binary or executing the binary? I'm a little confused because of the singleton; does synchronizing that method actually prevent two threads from attempting to execute the binary at the same time? When does the first thread relinquish the lock? |
diemol
left a comment
There was a problem hiding this comment.
LGTM!
Failing tests are not related to this change.
Description
The method to get the Selenium Manager binary in the Java bindings is not synchronized, which leading to concurrent problems to get Selenium Manager in Java, as reported in #11620. I have confirmed the bug using the provided example repo. I used that repo as well to test the fix.
Motivation and Context
This PR fixes #11620.
Types of changes
Checklist