diff --git a/playwright/src/main/java/com/microsoft/playwright/Playwright.java b/playwright/src/main/java/com/microsoft/playwright/Playwright.java index 2a09850d..beccabf2 100644 --- a/playwright/src/main/java/com/microsoft/playwright/Playwright.java +++ b/playwright/src/main/java/com/microsoft/playwright/Playwright.java @@ -1,12 +1,12 @@ /* * Copyright (c) Microsoft Corporation. - *

+ * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - *

+ * * http://www.apache.org/licenses/LICENSE-2.0 - *

+ * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -18,10 +18,9 @@ package com.microsoft.playwright; import com.microsoft.playwright.impl.PlaywrightImpl; -import java.io.IOException; import java.util.Map; -public interface Playwright { +public interface Playwright extends AutoCloseable { static Playwright create() { return PlaywrightImpl.create(); } @@ -33,4 +32,7 @@ public interface Playwright { Map devices(); Selectors selectors(); + + @Override + void close() throws Exception; } diff --git a/playwright/src/main/java/com/microsoft/playwright/example/Main.java b/playwright/src/main/java/com/microsoft/playwright/example/Main.java index bafc5349..dd904cbb 100644 --- a/playwright/src/main/java/com/microsoft/playwright/example/Main.java +++ b/playwright/src/main/java/com/microsoft/playwright/example/Main.java @@ -21,7 +21,7 @@ import java.io.File; import java.nio.file.Paths; public class Main { - public static void main(String[] args) { + public static void main(String[] args) throws Exception { Playwright playwright = Playwright.create(); Browser browser = playwright.chromium().launch(); BrowserContext context = browser.newContext( @@ -31,5 +31,6 @@ public class Main { page.click("text=check feature status"); page.screenshot(new Page.ScreenshotOptions().withPath(Paths.get("s.png"))); browser.close(); + playwright.close(); } } diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/PlaywrightImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/PlaywrightImpl.java index 828adac0..6ecfc8d5 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/PlaywrightImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/PlaywrightImpl.java @@ -29,44 +29,54 @@ import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.*; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; public class PlaywrightImpl extends ChannelOwner implements Playwright { private static Path driverTempDir; + private Process driverProcess; public static PlaywrightImpl create() { try { - Path driver = ensureDriverExtracted(); + Path driver = ensureDriverInstalled(); ProcessBuilder pb = new ProcessBuilder(driver.toString(), "run-driver"); pb.redirectError(ProcessBuilder.Redirect.INHERIT); // pb.environment().put("DEBUG", "pw:pro*"); Process p = pb.start(); Connection connection = new Connection(p.getInputStream(), p.getOutputStream()); - return (PlaywrightImpl) connection.waitForObjectWithKnownName("Playwright"); - } catch (IOException e) { + PlaywrightImpl result = (PlaywrightImpl) connection.waitForObjectWithKnownName("Playwright"); + result.driverProcess = p; + return result; + } catch (IOException | InterruptedException | URISyntaxException e) { throw new PlaywrightException("Failed to launch driver", e); } } - private static Path ensureDriverExtracted() { + private static synchronized Path ensureDriverInstalled() throws IOException, InterruptedException, URISyntaxException { if (driverTempDir == null) { - try { - driverTempDir = Files.createTempDirectory("playwright-java-"); - driverTempDir.toFile().deleteOnExit(); - ClassLoader classloader = Thread.currentThread().getContextClassLoader(); - Path path = Paths.get(classloader.getResource("driver").toURI()); - Files.list(path).forEach(filePath -> { - try { - extractResource(filePath, driverTempDir); - } catch (IOException e) { - throw new RuntimeException("Failed to extract driver from " + path, e); - } - }); - } catch (IOException | URISyntaxException e) { - throw new PlaywrightException("Failed to launch driver", e); + driverTempDir = Files.createTempDirectory("playwright-java-"); + driverTempDir.toFile().deleteOnExit(); + ClassLoader classloader = Thread.currentThread().getContextClassLoader(); + Path path = Paths.get(classloader.getResource("driver").toURI()); + Files.list(path).forEach(filePath -> { + try { + extractResource(filePath, driverTempDir); + } catch (IOException e) { + throw new PlaywrightException("Failed to extract driver from " + path, e); + } + }); + + Path driver = driverTempDir.resolve("playwright-cli"); + ProcessBuilder pb = new ProcessBuilder(driver.toString(), "install"); + pb.redirectError(ProcessBuilder.Redirect.INHERIT); + pb.redirectOutput(ProcessBuilder.Redirect.INHERIT); + Process p = pb.start(); + boolean result = p.waitFor(10, TimeUnit.MINUTES); + if (!result) { + System.err.println("Timed out waiting for browsers to install"); } } - // TODO: remove dir on exit return driverTempDir.resolve("playwright-cli"); } @@ -75,7 +85,7 @@ public class PlaywrightImpl extends ChannelOwner implements Playwright { Files.copy(from, path); path.toFile().setExecutable(true); path.toFile().deleteOnExit(); - System.out.println("extracting: " + from.toString() + " to " + path.toString()); +// System.out.println("extracting: " + from.toString() + " to " + path.toString()); return path; } @@ -85,7 +95,7 @@ public class PlaywrightImpl extends ChannelOwner implements Playwright { private final Selectors selectors; private final Map devices = new HashMap<>(); - public PlaywrightImpl(ChannelOwner parent, String type, String guid, JsonObject initializer) { + PlaywrightImpl(ChannelOwner parent, String type, String guid, JsonObject initializer) { super(parent, type, guid, initializer); chromium = parent.connection.getExistingObject(initializer.getAsJsonObject("chromium").get("guid").getAsString()); firefox = parent.connection.getExistingObject(initializer.getAsJsonObject("firefox").get("guid").getAsString()); @@ -126,11 +136,13 @@ public class PlaywrightImpl extends ChannelOwner implements Playwright { return selectors; } - public void close() { - try { - connection.close(); - } catch (IOException e) { - throw new PlaywrightException("Failed to close", e); + @Override + public void close() throws Exception { + connection.close(); + // playwright-cli will exit when its stdin is closed, we wait for that. + boolean didClose = driverProcess.waitFor(30, TimeUnit.SECONDS); + if (!didClose) { + System.err.println("WARNING: Timed out while waiting for driver process to exit"); } } } diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/Transport.java b/playwright/src/main/java/com/microsoft/playwright/impl/Transport.java index c117bb60..6e4de7d4 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/Transport.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/Transport.java @@ -69,16 +69,19 @@ public class Transport { return; } isClosed = true; - readerThread.interrupt(); - readerThread.in.close(); - writerThread.interrupt(); + // We interrupt only the outgoing pipe and keep reader thread running as + // otherwise child process may block on writing to its stdout and never + // exit (observed on Windows). + readerThread.isClosing = true; writerThread.out.close(); + writerThread.interrupt(); } } class ReaderThread extends Thread { - final DataInputStream in; + private final DataInputStream in; private final BlockingQueue queue; + volatile boolean isClosing; private static int readIntLE(DataInputStream in) throws IOException { int ch1 = in.read(); @@ -103,8 +106,9 @@ class ReaderThread extends Thread { try { queue.put(readMessage()); } catch (IOException e) { - if (!isInterrupted()) + if (!isInterrupted() && !isClosing) { e.printStackTrace(); + } break; } catch (InterruptedException e) { break; diff --git a/playwright/src/test/java/com/microsoft/playwright/TestBase.java b/playwright/src/test/java/com/microsoft/playwright/TestBase.java index 81b7551d..1e8f54b5 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestBase.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestBase.java @@ -69,6 +69,12 @@ public class TestBase { httpsServer = null; } + @AfterAll + static void closePlaywright() throws Exception { + playwright.close(); + playwright = null; + } + @BeforeEach void createContextAndPage() { server.reset();