« Return to Thread: [rfc][icedtea-web] try to close before kill

[rfc][icedtea-web] try to close before kill

by Jiri Vanek :: Rate this Message:

| View in Thread

hi!

This patch is trying to sigquite process before kill it. It can solve some of the issues we have caused by ff/chrom* caches,safe modes, restoration mechanisms and similar stuff.

What do you think?

 From fixed applet patch it can be seen that it behaves much better now.

What do you think?


J.

[trytoCloseBeforeKill.diff]

diff -r f7191afb7ab9 tests/reproducers/simple/AppletTest/testcases/AppletTestTests.java
--- a/tests/reproducers/simple/AppletTest/testcases/AppletTestTests.java Tue Jul 03 17:00:49 2012 -0400
+++ b/tests/reproducers/simple/AppletTest/testcases/AppletTestTests.java Wed Jul 04 13:18:10 2012 +0200
@@ -65,12 +65,12 @@
                         pr1.deadlyException.getMessage().contains(ServerAccess.UNSET_BROWSER));
                 return;
             }
-            evaluateApplet(pr1);
+            evaluateApplet(pr1,false);
             Assert.assertTrue(pr1.wasTerminated);
             //System.out.println("connecting AppletInFirefoxTest request in " + getBrowser().toString());
             // just verify loging is recording browser
             ServerAccess.ProcessResult pr = server.executeBrowser("/appletAutoTests.html");
-            evaluateApplet(pr);
+            evaluateApplet(pr,false);
             Assert.assertTrue(pr.wasTerminated);
         } finally {
             server.PROCESS_TIMEOUT = 20 * 1000; //back to normal
@@ -81,12 +81,12 @@
     @NeedsDisplay
     public void AppletTest() throws Exception {
         ServerAccess.ProcessResult pr = server.executeJavawsHeadless(null, "/AppletTest.jnlp");
-        evaluateApplet(pr);
+        evaluateApplet(pr,true);
         Assert.assertFalse(pr.wasTerminated);
         Assert.assertEquals((Integer) 0, pr.returnValue);
     }
 
-    private void evaluateApplet(ProcessResult pr) {
+    private void evaluateApplet(ProcessResult pr, boolean javawsApplet) {
         String s3 = "applet was initialised";
         Assert.assertTrue("AppletTest stdout should contains " + s3 + " bud didn't", pr.stdout.contains(s3));
         String s0 = "applet was started";
@@ -95,14 +95,16 @@
         Assert.assertTrue("AppletTest stdout should contains " + s1 + " bud didn't", pr.stdout.contains(s1));
         String s2 = "value2";
         Assert.assertTrue("AppletTest stdout should contains " + s2 + " bud didn't", pr.stdout.contains(s2));
-        String s4 = "applet was stopped";
-        Assert.assertFalse("AppletTest stdout shouldn't contains " + s4 + " bud did", pr.stdout.contains(s4));
-        String s5 = "applet will be destroyed";
-        Assert.assertFalse("AppletTest stdout shouldn't contains " + s5 + " bud did", pr.stdout.contains(s5));
         String ss = "xception";
         Assert.assertFalse("AppletTest stderr should not contains " + ss + " but did", pr.stderr.contains(ss));
         String s7 = "Aplet killing himself after 2000 ms of life";
         Assert.assertTrue("AppletTest stdout should contains " + s7 + " bud didn't", pr.stdout.contains(s7));
+        if (!javawsApplet) {
+        String s4 = "applet was stopped";
+        Assert.assertTrue("AppletTest stdout shouldn contains " + s4 + " bud did't", pr.stdout.contains(s4));
+        String s5 = "applet will be destroyed";
+        Assert.assertTrue("AppletTest stdout shouldn contains " + s5 + " bud did't", pr.stdout.contains(s5));
+        }
     }
 
     @Test
@@ -114,7 +116,7 @@
         ServerAccess.PROCESS_TIMEOUT = 30 * 1000;
         try {
             ServerAccess.ProcessResult pr = server.executeBrowser("/appletAutoTests2.html");
-            evaluateApplet(pr);
+            evaluateApplet(pr,false);
             Assert.assertTrue(pr.wasTerminated);
             //Assert.assertEquals((Integer) 0, pr.returnValue); due to destroy is null
         } finally {
@@ -131,7 +133,7 @@
         try {
             ServerAccess.ProcessResult pr = server.executeBrowser("/appletAutoTests.html");
             pr.process.destroy();
-            evaluateApplet(pr);
+            evaluateApplet(pr,false);
             Assert.assertTrue(pr.wasTerminated);
             //Assert.assertEquals((Integer) 0, pr.returnValue); due to destroy is null
         } finally {
diff -r f7191afb7ab9 tests/test-extensions/net/sourceforge/jnlp/ProcessAssasin.java
--- a/tests/test-extensions/net/sourceforge/jnlp/ProcessAssasin.java Tue Jul 03 17:00:49 2012 -0400
+++ b/tests/test-extensions/net/sourceforge/jnlp/ProcessAssasin.java Wed Jul 04 13:18:10 2012 +0200
@@ -34,8 +34,11 @@
 obligated to do so.  If you do not wish to do so, delete this
 exception statement from your version.
  */
+package net.sourceforge.jnlp;
 
-package net.sourceforge.jnlp;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * class which timeout any ThreadedProcess. This killing of 'thread with process' replaced not working process.destroy().
@@ -112,14 +115,10 @@
                                 ServerAccess.logErrorReprint("Timed out " + p.toString() + " " + "null  .. killing " + p.getCommandLine() + ": ");
                             }
                             wasTerminated = true;
-                            p.interrupt();
-                            while (!ServerAccess.terminated.contains(p)) {
-                                Thread.sleep(100);
-                            }
                             if (p.getP() != null) {
                                 try {
                                     if (!skipInstedOfDesroy) {
-                                        p.getP().destroy();
+                                        destroyProcess(p);
                                     }
                                 } catch (Throwable ex) {
                                     if (p.deadlyException == null) {
@@ -128,6 +127,10 @@
                                     ex.printStackTrace();
                                 }
                             }
+                            p.interrupt();
+//                            while (!ServerAccess.terminated.contains(p)) {
+//                                Thread.sleep(100);
+//                            }
                             if (p.getP() != null) {
                                 ServerAccess.logErrorReprint("Timed out " + p.toString() + " " + p.getP().toString() + " .. killed " + p.getCommandLine());
                             } else {
@@ -156,4 +159,24 @@
             ServerAccess.logNoReprint("assassin for non existing job  termination " + wasTerminated);
         }
     }
+
+    public static void destroyProcess(ThreadedProcess pp) {
+        Process p = pp.getP();
+        try {
+            Field f = p.getClass().getDeclaredField("pid");
+            f.setAccessible(true);
+            String pid = (f.get(p)).toString();
+            List<String> ll=new ArrayList<String>(4);
+            ll.add("kill");
+            ll.add("-s");
+            ll.add("SIGQUIT");
+            ll.add(pid);
+            ServerAccess.executeProcess(ll);
+            Thread.sleep(500);
+        } catch (Exception ex) {
+            ServerAccess.logException(ex);
+        } finally {
+            p.destroy();
+        }
+    }
 }
diff -r f7191afb7ab9 tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java
--- a/tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java Tue Jul 03 17:00:49 2012 -0400
+++ b/tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java Wed Jul 04 13:18:10 2012 +0200
@@ -111,14 +111,10 @@
      */
     public static long PROCESS_TIMEOUT = 20 * 1000;//ms
     /**
-     * all terminated processes are stored here. As wee need to 'wait' to termination to be finished.
-     */
-    static Set<Thread> terminated = new HashSet<Thread>();
-    /**
      * this flag is indicating whether output of executeProcess should be logged. By default true.
      */
     public static boolean PROCESS_LOG = true;
-    public static boolean LOGS_REPRINT = false;
+    public static boolean LOGS_REPRINT = false;
 
     private Browser currentBrowser;
     public static final String UNSET_BROWSER="unset_browser";
diff -r f7191afb7ab9 tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java
--- a/tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java Tue Jul 03 17:00:49 2012 -0400
+++ b/tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java Wed Jul 04 13:18:10 2012 +0200
@@ -125,10 +125,10 @@
                 //add to the set of terminated threaded processes
                 deadlyException = ex;
                 ServerAccess.logException(deadlyException, false);
-                ServerAccess.terminated.add(this);
+                //ServerAccess.terminated.add(this);
             } else {
                 //happens when non-existing process is launched, is causing p null!
-                ServerAccess.terminated.add(this);
+                //ServerAccess.terminated.add(this);
                 deadlyException = ex;
                 ServerAccess.logException(deadlyException, false);
                 throw new RuntimeException(ex);

 « Return to Thread: [rfc][icedtea-web] try to close before kill