From 0fa416b45931a3db254665f84a5c8375edf67b58 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 6 May 2021 02:24:57 +0000 Subject: [PATCH] fix: manually pipe messages from child process sdtout/stderr (#426) --- driver-bundle/pom.xml | 4 +- .../microsoft/playwright/impl/DriverJar.java | 3 + .../com/microsoft/playwright/TestInstall.java | 7 +- driver/pom.xml | 3 + .../playwright/impl/StreamRedirectThread.java | 69 +++++++++++++++++++ .../playwright/impl/PipeTransport.java | 14 +++- .../playwright/impl/PlaywrightImpl.java | 11 ++- 7 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 driver/src/main/java/com/microsoft/playwright/impl/StreamRedirectThread.java diff --git a/driver-bundle/pom.xml b/driver-bundle/pom.xml index e7ff740f6..be8ccbdef 100644 --- a/driver-bundle/pom.xml +++ b/driver-bundle/pom.xml @@ -44,7 +44,9 @@ org.apache.maven.plugins maven-surefire-plugin - 3.0.0-M5 + + true + diff --git a/driver-bundle/src/main/java/com/microsoft/playwright/impl/DriverJar.java b/driver-bundle/src/main/java/com/microsoft/playwright/impl/DriverJar.java index 305eca1ad..0f35f9ccd 100644 --- a/driver-bundle/src/main/java/com/microsoft/playwright/impl/DriverJar.java +++ b/driver-bundle/src/main/java/com/microsoft/playwright/impl/DriverJar.java @@ -48,6 +48,9 @@ private void installBrowsers() throws IOException, InterruptedException { p.destroy(); throw new RuntimeException("Timed out waiting for browsers to install"); } + if (p.exitValue() != 0) { + throw new RuntimeException("Failed to install browsers, exit code: " + p.exitValue()); + } } private static boolean isExecutable(Path filePath) { diff --git a/driver-bundle/src/test/java/com/microsoft/playwright/TestInstall.java b/driver-bundle/src/test/java/com/microsoft/playwright/TestInstall.java index 6c28f71b3..d363b0c64 100644 --- a/driver-bundle/src/test/java/com/microsoft/playwright/TestInstall.java +++ b/driver-bundle/src/test/java/com/microsoft/playwright/TestInstall.java @@ -17,6 +17,7 @@ package com.microsoft.playwright; import com.microsoft.playwright.impl.Driver; +import com.microsoft.playwright.impl.StreamRedirectThread; import org.junit.jupiter.api.Test; import java.nio.file.Files; @@ -36,11 +37,13 @@ void playwrightCliInstalled() throws Exception { assertTrue(Files.exists(cli)); ProcessBuilder pb = new ProcessBuilder(cli.toString(), "install"); - pb.redirectError(ProcessBuilder.Redirect.INHERIT); - pb.redirectOutput(ProcessBuilder.Redirect.INHERIT); Process p = pb.start(); + StreamRedirectThread stdoutThread = new StreamRedirectThread(p.getInputStream(), System.out); + StreamRedirectThread stderrThread = new StreamRedirectThread(p.getErrorStream(), System.err); boolean result = p.waitFor(1, TimeUnit.MINUTES); assertTrue(result, "Timed out waiting for browsers to install"); + stderrThread.terminateAndJoin(); + stdoutThread.terminateAndJoin(); } catch (Exception e) { e.printStackTrace(); assertNull(e); diff --git a/driver/pom.xml b/driver/pom.xml index d1754c0e3..1cd23ea75 100644 --- a/driver/pom.xml +++ b/driver/pom.xml @@ -40,6 +40,9 @@ org.apache.maven.plugins maven-surefire-plugin + + true + diff --git a/driver/src/main/java/com/microsoft/playwright/impl/StreamRedirectThread.java b/driver/src/main/java/com/microsoft/playwright/impl/StreamRedirectThread.java new file mode 100644 index 000000000..1d1179fb1 --- /dev/null +++ b/driver/src/main/java/com/microsoft/playwright/impl/StreamRedirectThread.java @@ -0,0 +1,69 @@ +/* + * 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. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.microsoft.playwright.impl; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +// We manually copy stderr and stdout from child process as INHERIT for err/out streams +// doesn't work well in Java Enterprise, see +// https://github.com/microsoft/playwright-java/issues/418#issuecomment-832650650 +public class StreamRedirectThread extends Thread { + private final InputStream from; + private final OutputStream to; + private volatile boolean terminated; + + public StreamRedirectThread(InputStream from, OutputStream to) { + this.from = from; + this.to = to; + start(); + } + + @Override + public void run() { + byte[] buffer = new byte[1<<14]; + try { + while (true) { + while (from.available() != 0) { + int len = from.read(buffer); + if (len != -1) { + to.write(buffer); + } + } + if (terminated) { + break; + } + try { + Thread.sleep(100); + } catch (InterruptedException e) { + } + } + } catch (IOException e) { + e.printStackTrace(System.err); + } + } + + public void terminateAndJoin() { + terminated = true; + try { + join(); + } catch (InterruptedException e) { + e.printStackTrace(System.err); + } + } +} diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/PipeTransport.java b/playwright/src/main/java/com/microsoft/playwright/impl/PipeTransport.java index 4fe86f650..8257bf30c 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/PipeTransport.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/PipeTransport.java @@ -59,7 +59,16 @@ public String poll(Duration timeout) { throw new PlaywrightException("Playwright connection closed"); } try { - return incoming.poll(timeout.toMillis(), TimeUnit.MILLISECONDS); + String message = incoming.poll(timeout.toMillis(), TimeUnit.MILLISECONDS); + if (message == null && readerThread.exception != null) { + try { + close(); + } catch (IOException e) { + e.printStackTrace(System.err); + } + throw new PlaywrightException("Failed to read message from driver, pipe closed.", readerThread.exception); + } + return message; } catch (InterruptedException e) { throw new PlaywrightException("Failed to read message", e); } @@ -84,6 +93,7 @@ class ReaderThread extends Thread { private final DataInputStream in; private final BlockingQueue queue; volatile boolean isClosing; + volatile Exception exception; private static int readIntLE(DataInputStream in) throws IOException { int ch1 = in.read(); @@ -109,7 +119,7 @@ public void run() { queue.put(readMessage()); } catch (IOException e) { if (!isInterrupted() && !isClosing) { - e.printStackTrace(); + exception = e; } break; } catch (InterruptedException e) { 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 ecb19da56..435bc9b66 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/PlaywrightImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/PlaywrightImpl.java @@ -27,21 +27,29 @@ public class PlaywrightImpl extends ChannelOwner implements Playwright { private Process driverProcess; + private StreamRedirectThread stderrThread; public static PlaywrightImpl create() { + StreamRedirectThread stderrThread = null; try { Path driver = 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(); + stderrThread = new StreamRedirectThread(p.getErrorStream(), System.err); Connection connection = new Connection(new PipeTransport(p.getInputStream(), p.getOutputStream())); PlaywrightImpl result = (PlaywrightImpl) connection.waitForObjectWithKnownName("Playwright"); result.driverProcess = p; + result.stderrThread = stderrThread; + stderrThread = null; result.initSharedSelectors(null); return result; } catch (IOException e) { throw new PlaywrightException("Failed to launch driver", e); + } finally { + if (stderrThread != null) { + stderrThread.terminateAndJoin(); + } } } @@ -103,6 +111,7 @@ public void close() { if (!didClose) { System.err.println("WARNING: Timed out while waiting for driver process to exit"); } + stderrThread.terminateAndJoin(); } catch (IOException e) { throw new PlaywrightException("Failed to terminate", e); } catch (InterruptedException e) {