/hg/icedtea-web: runtime.exec replaced by ProcessBuilder. All wa...

jvanek at icedtea.classpath.org jvanek at icedtea.classpath.org
Tue Jun 16 10:12:19 UTC 2015


changeset 37dfc20a1816 in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=37dfc20a1816
author: Jiri Vanek <jvanek at redhat.com>
date: Tue Jun 16 12:11:58 2015 +0200

	runtime.exec replaced by ProcessBuilder. All waits for proeces amde safe

	* netx/net/sourceforge/jnlp/Launcher.java: runtime.exec replaced by process builder. All streams correctly redirected
	* netx/net/sourceforge/jnlp/util/XDesktopEntry.java: same
	* netx/net/sourceforge/jnlp/StreamEater.java: no longer used, removed
	* netx/net/sourceforge/jnlp/util/StreamUtils.java: Added new utility method waitForSafely
	* netx/net/sourceforge/jnlp/controlpanel/JVMPanel.java: now uses waitForSafely
	* netx/net/sourceforge/jnlp/services/XBasicService.java: same
	* netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java: same
	* tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java: same
	* netx/net/sourceforge/jnlp/util/logging/TeeOutputStream.java: cleaned whitespaces


diffstat:

 ChangeLog                                                       |  16 ++
 netx/net/sourceforge/jnlp/Launcher.java                         |  25 +--
 netx/net/sourceforge/jnlp/StreamEater.java                      |  56 ----------
 netx/net/sourceforge/jnlp/controlpanel/JVMPanel.java            |   2 +-
 netx/net/sourceforge/jnlp/services/XBasicService.java           |   6 +-
 netx/net/sourceforge/jnlp/util/StreamUtils.java                 |  22 +++
 netx/net/sourceforge/jnlp/util/XDesktopEntry.java               |  13 +-
 netx/net/sourceforge/jnlp/util/logging/TeeOutputStream.java     |   3 +-
 netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java       |   3 +-
 tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java |   4 +-
 10 files changed, 62 insertions(+), 88 deletions(-)

diffs (312 lines):

diff -r 78b33f42aa24 -r 37dfc20a1816 ChangeLog
--- a/ChangeLog	Tue Jun 16 11:29:01 2015 +0200
+++ b/ChangeLog	Tue Jun 16 12:11:58 2015 +0200
@@ -1,3 +1,19 @@
+2015-06-16  Jiri Vanek  <jvanek at redhat.com>
+
+	runtime.exec replaced by ProcessBuilder. All waits for proeces amde safe
+	* netx/net/sourceforge/jnlp/Launcher.java: runtime.exec replaced by process builder.
+	All streams correctly redirected
+	* netx/net/sourceforge/jnlp/util/XDesktopEntry.java: same
+	* netx/net/sourceforge/jnlp/StreamEater.java: no longer used, removed
+	* netx/net/sourceforge/jnlp/util/StreamUtils.java: Added new utility method
+	waitForSafely
+	* netx/net/sourceforge/jnlp/controlpanel/JVMPanel.java: now uses waitForSafely
+	* netx/net/sourceforge/jnlp/services/XBasicService.java: same
+	* netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java: same
+	* tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java: same
+	* netx/net/sourceforge/jnlp/util/logging/TeeOutputStream.java: cleaned whitespaces
+
+
 2015-06-16  Jiri Vanek  <jvanek at redhat.com>
 
 	Removed dual panel in panel misleading declaration in AppTrustWarningDialog
diff -r 78b33f42aa24 -r 37dfc20a1816 netx/net/sourceforge/jnlp/Launcher.java
--- a/netx/net/sourceforge/jnlp/Launcher.java	Tue Jun 16 11:29:01 2015 +0200
+++ b/netx/net/sourceforge/jnlp/Launcher.java	Tue Jun 16 12:11:58 2015 +0200
@@ -43,6 +43,7 @@
 import javax.swing.SwingUtilities;
 import javax.swing.text.html.parser.ParserDelegator;
 import net.sourceforge.jnlp.splashscreen.SplashUtils;
+import net.sourceforge.jnlp.util.StreamUtils;
 import net.sourceforge.jnlp.util.logging.OutputController;
 
 import sun.awt.SunToolkit;
@@ -368,9 +369,8 @@
 
   
     /**
-     * Launches the JNLP file in a new JVM instance.  The launched
-     * application's output is sent to the system out and it's
-     * standard input channel is closed.
+     * Launches the JNLP file in a new JVM instance. 
+     * All streams are properly redirected.
      *
      * @param vmArgs the arguments to pass to the new JVM. Can be empty but
      *        must not be null.
@@ -397,9 +397,8 @@
     }
 
     /**
-     * Launches the JNLP file in a new JVM instance.  The launched
-     * application's output is sent to the system out and it's
-     * standard input channel is closed.
+     * Launches the JNLP file in a new JVM instance.
+     * All streams are properly redirected.
      *
      * @param url the URL of the JNLP file to launch
      * @throws LaunchException if there was an exception
@@ -412,8 +411,7 @@
 
     /**
      * Launches the JNLP file at the specified location in a new JVM
-     * instance.  The launched application's output is sent to the
-     * system out and it's standard input channel is closed.
+     * instance. All streams are properly redirected.
      * @param vmArgs the arguments to pass to the jvm
      * @param javawsArgs the arguments to pass to javaws (aka Netx)
      * @throws LaunchException if there was an exception
@@ -421,7 +419,7 @@
     public void launchExternal(List<String> vmArgs, List<String> javawsArgs) throws LaunchException {
         try {
 
-            List<String> commands = new LinkedList<String>();
+            List<String> commands = new LinkedList<>();
 
             // this property is set by the javaws launcher to point to the javaws binary
             String pathToWebstartBinary = System.getProperty("icedtea-web.bin.location");
@@ -434,11 +432,10 @@
 
             String[] command = commands.toArray(new String[] {});
 
-            Process p = Runtime.getRuntime().exec(command);
-            new StreamEater(p.getErrorStream()).start();
-            new StreamEater(p.getInputStream()).start();
-            p.getOutputStream().close();
-
+            ProcessBuilder pb = new ProcessBuilder(command);
+            pb.inheritIO();
+            Process p =pb.start();
+            StreamUtils.waitForSafely(p);
         } catch (NullPointerException ex) {
             throw launchError(new LaunchException(null, null, R("LSFatal"), R("LCExternalLaunch"), R("LNetxJarMissing"), R("LNetxJarMissingInfo")));
         } catch (Exception ex) {
diff -r 78b33f42aa24 -r 37dfc20a1816 netx/net/sourceforge/jnlp/StreamEater.java
--- a/netx/net/sourceforge/jnlp/StreamEater.java	Tue Jun 16 11:29:01 2015 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,56 +0,0 @@
-// Copyright (C) 2001-2003 Jon A. Maxwell (JAM)
-//
-// This library is free software; you can redistribute it and/or
-// modify it under the terms of the GNU Lesser General Public
-// License as published by the Free Software Foundation; either
-// version 2.1 of the License, or (at your option) any later version.
-//
-// This library is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-// Lesser General Public License for more details.
-//
-// You should have received a copy of the GNU Lesser General Public
-// License along with this library; if not, write to the Free Software
-// Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
-
-package net.sourceforge.jnlp;
-
-import java.io.BufferedInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import net.sourceforge.jnlp.util.logging.OutputController;
-
-/**
- * This class reads the output from a launched process and writes it to stdout.
- */
-public class StreamEater extends Thread {
-    private InputStream stream;
-
-    public StreamEater(InputStream stream) {
-        this.stream = new BufferedInputStream(stream);
-    }
-
-    public void run() {
-        try {
-            StringBuilder s = new StringBuilder();
-            while (true) {
-                int c = stream.read();
-                if (c == -1){
-                    OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, s.toString());
-                    break;
-                } else {
-                    char ch = (char) c;
-                    if (ch == '\n'){
-                        OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, s.toString());
-                        s = new StringBuilder();
-                    }else {
-                        s.append((char) c);
-                    }
-                }
-                
-            }
-        } catch (IOException ex) {
-        }
-    }
-}
diff -r 78b33f42aa24 -r 37dfc20a1816 netx/net/sourceforge/jnlp/controlpanel/JVMPanel.java
--- a/netx/net/sourceforge/jnlp/controlpanel/JVMPanel.java	Tue Jun 16 11:29:01 2015 +0200
+++ b/netx/net/sourceforge/jnlp/controlpanel/JVMPanel.java	Tue Jun 16 12:11:58 2015 +0200
@@ -251,7 +251,7 @@
         Integer r = null;
         try {
             p = sb.start();
-            p.waitFor();
+            StreamUtils.waitForSafely(p);
             processErrorStream = StreamUtils.readStreamAsString(p.getErrorStream());
             processStdOutStream = StreamUtils.readStreamAsString(p.getInputStream());
             r = p.exitValue();
diff -r 78b33f42aa24 -r 37dfc20a1816 netx/net/sourceforge/jnlp/services/XBasicService.java
--- a/netx/net/sourceforge/jnlp/services/XBasicService.java	Tue Jun 16 11:29:01 2015 +0200
+++ b/netx/net/sourceforge/jnlp/services/XBasicService.java	Tue Jun 16 12:11:58 2015 +0200
@@ -35,6 +35,7 @@
 import net.sourceforge.jnlp.config.DeploymentConfiguration;
 import net.sourceforge.jnlp.runtime.ApplicationInstance;
 import net.sourceforge.jnlp.runtime.JNLPRuntime;
+import net.sourceforge.jnlp.util.StreamUtils;
 import net.sourceforge.jnlp.util.logging.OutputController;
 
 /**
@@ -268,14 +269,11 @@
 
         try {
             Process p = Runtime.getRuntime().exec(new String[] { "bash", "-c", "type " + command });
-            p.waitFor();
+            StreamUtils.waitForSafely(p);
             return (p.exitValue() == 0);
         } catch (IOException e) {
             OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
             return false;
-        } catch (InterruptedException e) {
-            OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
-            return false;
         }
     }
 
diff -r 78b33f42aa24 -r 37dfc20a1816 netx/net/sourceforge/jnlp/util/StreamUtils.java
--- a/netx/net/sourceforge/jnlp/util/StreamUtils.java	Tue Jun 16 11:29:01 2015 +0200
+++ b/netx/net/sourceforge/jnlp/util/StreamUtils.java	Tue Jun 16 12:11:58 2015 +0200
@@ -111,4 +111,26 @@
 
         return sb.toString();
     }
+    
+    /**
+     * This should be workaround for https://en.wikipedia.org/wiki/Spurious_wakeup which real can happen in case of processes.
+     * See http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2015-June/032350.html thread
+     * @param p process to be waited for
+     */
+    public static void waitForSafely(Process p) {
+        boolean pTerminated = false;
+        while (!pTerminated) {
+            try {
+                p.waitFor();
+            } catch (InterruptedException e) {
+                OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, e);
+            }
+            try {
+                p.exitValue();
+                pTerminated = true;
+            } catch (IllegalThreadStateException e) {
+                OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, e);
+            }
+        }
+    }
 }
diff -r 78b33f42aa24 -r 37dfc20a1816 netx/net/sourceforge/jnlp/util/XDesktopEntry.java
--- a/netx/net/sourceforge/jnlp/util/XDesktopEntry.java	Tue Jun 16 11:29:01 2015 +0200
+++ b/netx/net/sourceforge/jnlp/util/XDesktopEntry.java	Tue Jun 16 12:11:58 2015 +0200
@@ -42,7 +42,6 @@
 import net.sourceforge.jnlp.JNLPFile;
 import net.sourceforge.jnlp.OptionsDefinitions;
 import net.sourceforge.jnlp.PluginBridge;
-import net.sourceforge.jnlp.StreamEater;
 import net.sourceforge.jnlp.cache.CacheUtil;
 import net.sourceforge.jnlp.cache.UpdatePolicy;
 import net.sourceforge.jnlp.config.PathsAndFiles;
@@ -380,15 +379,11 @@
             String[] execString = new String[] { "xdg-desktop-icon", "install", "--novendor",
                     shortcutFile.getCanonicalPath() };
             OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "Execing: " + Arrays.toString(execString));
-            Process installer = Runtime.getRuntime().exec(execString);
-            new StreamEater(installer.getInputStream()).start();
-            new StreamEater(installer.getErrorStream()).start();
+            ProcessBuilder pb = new ProcessBuilder(execString);
+            pb.inheritIO();
+            Process installer = pb.start();
 
-            try {
-                installer.waitFor();
-            } catch (InterruptedException e) {
-                OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
-            }
+            StreamUtils.waitForSafely(installer);
 
             if (!shortcutFile.delete()) {
                 throw new IOException("Unable to delete temporary file:" + shortcutFile);
diff -r 78b33f42aa24 -r 37dfc20a1816 netx/net/sourceforge/jnlp/util/logging/TeeOutputStream.java
--- a/netx/net/sourceforge/jnlp/util/logging/TeeOutputStream.java	Tue Jun 16 11:29:01 2015 +0200
+++ b/netx/net/sourceforge/jnlp/util/logging/TeeOutputStream.java	Tue Jun 16 12:11:58 2015 +0200
@@ -56,7 +56,7 @@
     // Everthing written to TeeOutputStream is written to our log too
     private final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
     private final boolean isError;
-    
+
     public TeeOutputStream(PrintStream stdStream, boolean isError) {
         super(stdStream);
         this.isError = isError;
@@ -142,5 +142,4 @@
         copy.write(this.byteArrayOutputStream.toByteArray());
         return copy;
     }
-
 }
diff -r 78b33f42aa24 -r 37dfc20a1816 netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java
--- a/netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java	Tue Jun 16 11:29:01 2015 +0200
+++ b/netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java	Tue Jun 16 12:11:58 2015 +0200
@@ -36,6 +36,7 @@
 
 package net.sourceforge.jnlp.util.logging;
 
+import net.sourceforge.jnlp.util.StreamUtils;
 import net.sourceforge.jnlp.util.docprovider.TextsProvider;
 
 
@@ -56,7 +57,7 @@
                 m = m.replaceAll("\t", "    ");
                 ProcessBuilder pb = new ProcessBuilder("logger", "-p","user.err", "--", m);
                 Process p = pb.start();
-                p.waitFor();
+                StreamUtils.waitForSafely(p);
                 OutputController.getLogger().log("System logger called with result of " + p.exitValue());
             }
         } catch (Exception ex) {
diff -r 78b33f42aa24 -r 37dfc20a1816 tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java
--- a/tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java	Tue Jun 16 11:29:01 2015 +0200
+++ b/tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java	Tue Jun 16 12:11:58 2015 +0200
@@ -39,6 +39,7 @@
 
 import java.io.File;
 import java.util.List;
+import net.sourceforge.jnlp.util.StreamUtils;
 
 /**
  *
@@ -141,7 +142,8 @@
                 p = r.exec(args.toArray(new String[0]), variables, dir);
             }
             try {
-                exitCode = p.waitFor();
+                StreamUtils.waitForSafely(p);
+                exitCode = p.exitValue();
                 Thread.sleep(500); //this is giving to fast done proecesses's e/o readers time to read all. I would like to know better solution :-/
                 while(assasin.isKilling() && !assasin.haveKilled()){
                     Thread.sleep(100);


More information about the distro-pkg-dev mailing list