AppletStub clean-ups

Koos Vriezen koos.vriezen at xs4all.nl
Sat Oct 2 15:49:09 BST 2004


Hi,

Fixing map24.com, I found a deadlock in AppletStub.stopApplet() when
using the KJASURLStreamHandlerFactory. Like other methods in AppletStub,
they can trigger an URL request and so lock up the protocol manager if
this call isn't done in a seperate thread.
Now we have already a runThread, an appletThread and a destroyThread.
Not to mention that this is racy as hell (see all the synchronized lines
that are often commented out because of dead locks).
So instead of introducing a new stopThread I thought why not just fix it
:-). The idea is to do all applet calls in one thread now and thread
safe.
Comments?

Koos
-------------- next part --------------
Index: org/kde/kjas/server/KJASAppletStub.java
===================================================================
RCS file: /home/kde/kdelibs/khtml/java/org/kde/kjas/server/KJASAppletStub.java,v
retrieving revision 1.57
diff -u -3 -p -r1.57 KJASAppletStub.java
--- org/kde/kjas/server/KJASAppletStub.java	14 May 2004 15:38:16 -0000	1.57
+++ org/kde/kjas/server/KJASAppletStub.java	2 Oct 2004 14:25:56 -0000
@@ -58,17 +58,142 @@ public final class KJASAppletStub
     * the applet has been destroyed 
     */
     public static final int DESTROYED = 6;
+    /**
+    * request for termination of the applet thread 
+    */
+    public static final int TERMINATE = 7;
    
     
     private int state = UNKNOWN;  
     private KJASAppletClassLoader loader;
     private KJASAppletPanel       panel;
     private Applet                app;
-    private Thread                runThread;
-    private Thread                appletThread = null;
-    private Thread                destroyThread = null;
     KJASAppletStub                me;
 
+    private class RunThread extends Thread {
+        private int request_state = UNKNOWN;
+        private int current_state = UNKNOWN;
+
+        RunThread() {
+            super("KJAS-AppletStub-" + appletID + "-" + appletName);
+            setContextClassLoader(loader);
+        }
+        /**
+         * Ask applet to go to the next state
+         */
+        synchronized void requestState(int nstate) {
+            if (nstate > current_state) {
+                request_state = state;
+                notifyAll();
+            }
+        }
+        /**
+         * Get the asked state
+         */
+        synchronized int getRequestState() {
+            while (request_state == current_state) {
+                try {
+                    wait ();
+                } catch(InterruptedException ie) {
+                }
+            }
+            return request_state;
+        }
+        /**
+         * Put applet in asked state
+         */
+        private void doState(int nstate) throws ClassNotFoundException, IllegalAccessException, InstantiationException {
+            switch (nstate) {
+                case CLASS_LOADED:
+                    appletClass = loader.loadClass( className );
+                    requestState(INSTANCIATED);
+                    break;
+                case INSTANCIATED: {
+                    Object object = null;
+                    try {
+                        object = appletClass.newInstance();
+                        app = (Applet) object;
+                    }
+                    catch ( ClassCastException e ) {
+                        if ( object != null && object instanceof java.awt.Component) {
+                            app = new Applet();
+                            app.setLayout(new BorderLayout());
+                            app.add( (Component) object, BorderLayout.CENTER);
+                        } else
+                            throw e;
+                    }
+                    requestState(INITIALIZED);
+                    break;
+                }
+                case INITIALIZED:
+                    app.setStub( me );
+                    app.setVisible(false);
+                    panel.setApplet( app );
+                    if (appletSize.getWidth() > 0)
+                        app.setBounds( 0, 0, appletSize.width, appletSize.height );
+                    else
+                        app.setBounds( 0, 0, panel.getSize().width, panel.getSize().height );
+                    app.init();
+                    loader.removeStatusListener(panel);
+                    app.setVisible(true);
+                    // stop the loading... animation 
+                    panel.stopAnimation();
+                    break;
+                case STARTED:
+                    active = true;
+                    frame.validate();
+                    app.start();
+                    app.repaint();
+                    break;
+                case STOPPED:
+                    active = false;
+                    app.stop();
+                    if (Main.java_version > 1.399) {
+                        // kill the windowClosing listener(s)
+                        WindowListener[] l = frame.getWindowListeners();
+                        for (int i = 0; l != null && i < l.length; i++)
+                            frame.removeWindowListener(l[i]);
+                    }
+                    frame.hide();
+                    break;
+                case DESTROYED:
+                    app.destroy();
+                    frame.dispose();
+                    app = null;
+                    loader = null;
+                    requestState(TERMINATE);
+                    break;
+                default:
+                    return;
+                }
+                stateChange(nstate);
+        }
+        /**
+         * RunThread run(), loop until state is TERMINATE
+         */
+        public void run() {
+            while (true) {
+                int nstate = getRequestState();
+                if (nstate == TERMINATE)
+                    return;
+                try {
+                    doState(nstate);
+                } catch (Exception ex) {
+                    Main.kjas_err("Error during state " + nstate, ex);
+                    if (nstate < INITIALIZED) {
+                        setFailed(ex.toString());
+                        requestState(TERMINATE);
+                    }
+                } catch (Throwable tr) {
+                    setFailed(tr.toString());
+                    requestState(TERMINATE);
+                }
+                current_state = nstate;
+            }
+        }
+    }
+    private RunThread                runThread;
+
     /**
      * Create an AppletStub for the specified applet. The stub will be in
      * the specified context and will automatically attach itself to the
@@ -110,20 +235,6 @@ public final class KJASAppletStub
         
     }
 
-    /**
-     * Helper function for ending the runThread and appletThread
-     **/
-    private void tryToStopThread(Thread thread) {
-        try {
-            thread.interrupt();
-        } catch (SecurityException se) {}
-        if (thread.isAlive()) {
-            try {
-                thread.join(5000);
-            } catch (InterruptedException ie) {}
-        }
-    }
-
     private void stateChange(int newState) {
         if (failed)
             return;
@@ -170,104 +281,8 @@ public final class KJASAppletStub
             frame.setBounds( 0, 0, 50, 50 );
         frame.setVisible(true);
         loader.addStatusListener(panel);
-        runThread = new Thread
-        (
-        new Runnable() {
-            public void run() {
-                //this order is very important and took a long time
-                //to figure out- don't modify it unless there are
-                //real bug fixes
-                
-                // till 2002 09 18
-                // commented out the synchronized block because
-                // it leads to a deadlock of the JVM with
-                // j2re 1.4.1
-                //synchronized( me ) {
-                    try {
-                        appletClass = loader.loadClass( className );
-                    } catch (Exception e) {
-                        Main.kjas_err("Class could not be loaded: " + className, e);
-                        setFailed(e.toString());
-                        return;
-                    }
-                    if (Thread.interrupted())
-                        return;
-                    stateChange(CLASS_LOADED);
-                    Object object = null;
-                    try {
-                        synchronized (appletClass) {
-                           object = appletClass.newInstance();
-                           app = (Applet) object;
-                        }
-                    }
-                    catch( InstantiationException e ) {
-                        Main.kjas_err( "Could not instantiate applet", e );
-                        setFailed(e.toString());
-                        return;
-                    }
-                    catch( IllegalAccessException e ) {
-                        Main.kjas_err( "Could not instantiate applet", e );
-                        setFailed(e.toString());
-                        return;
-                    }
-                    catch ( ClassCastException e ) {
-                        if ( object != null && object instanceof java.awt.Component) {
-                            app = new Applet();
-                            app.setLayout(new BorderLayout());
-                            app.add( (Component) object, BorderLayout.CENTER);
-                        } else {
-                            setFailed(e.toString());
-                            return;
-                        }
-                    }
-                    catch ( Throwable e ) {
-                        setFailed(e.toString());
-                        return;
-                    }
-                //} // synchronized
-                if (Thread.interrupted())
-                    return;
-                app.setStub( me );
-                stateChange(INSTANCIATED);
-                app.setVisible(false);
-                panel.setApplet( app );
-                if (appletSize.getWidth() > 0)
-                    app.setBounds( 0, 0, appletSize.width, appletSize.height );
-                else
-                    app.setBounds( 0, 0, panel.getSize().width, panel.getSize().height );
-
-                try {
-                    app.init();
-                } catch (Error er) {
-                    Main.info("Error " + er.toString() + " during applet initialization"); 
-                    er.printStackTrace();
-                    setFailed(er.toString());
-                    return;
-                } catch (Exception ex) {
-                    Main.info("Exception " + ex.toString() + " during applet initialization"); 
-                    ex.printStackTrace();
-                    setFailed(ex.toString());
-                    return;
-                }
-
-                if (Thread.interrupted())
-                     return;
-                stateChange(INITIALIZED);
-                loader.removeStatusListener(panel);
-                app.setVisible(true);
-                //panel.validate();
-               
-                // stop the loading... animation 
-                panel.stopAnimation();
-                // create a new thread, so we know, when the applet was started
-                /*Thread appletThread = new KJASAppletThread(me, "KJAS-Applet-" + appletID + "-" + appletName); 
-                appletThread.start();
-                state = STARTED;
-                context.showStatus("Applet " + appletName + " started.");*/
-           }
-        }
-        , "KJAS-AppletStub-" + appletID + "-" + appletName);
-        runThread.setContextClassLoader(loader);
+        runThread = new RunThread();
+        runThread.requestState(CLASS_LOADED);
         runThread.start();
     }
 
@@ -280,25 +295,7 @@ public final class KJASAppletStub
     */
     void startApplet()
     {
-        if( app != null && state == INITIALIZED) {
-            active = true;
-            if (appletThread == null) {
-                appletThread = new Thread("KJAS-Applet-" + appletID + "-" + appletName) {
-                    public void run() {
-                        frame.validate();
-                        app.start();
-                        app.repaint();
-                        appletThread = null;
-                    }
-                };
-                appletThread.start();
-            } /*else {
-                frame.validate();
-                app.start();
-                app.repaint();
-            }*/
-            stateChange(STARTED);
-       }
+        runThread.requestState(STARTED);
     }
 
     /**
@@ -310,23 +307,7 @@ public final class KJASAppletStub
     */
     void stopApplet()
     {
-        if( app != null ) {
-            if( appletThread != null && appletThread.isAlive() ) {
-                Main.debug( "appletThread is active when stop is called" );
-                tryToStopThread(appletThread);
-            }
-            appletThread = null;
-            active = false;
-            app.stop();
-            stateChange(STOPPED);
-            if (Main.java_version > 1.399) {
-                // kill the windowClosing listener(s)
-                WindowListener[] wl = frame.getWindowListeners();
-                for (int i = 0; wl != null && i < wl.length; i++)
-                    frame.removeWindowListener(wl[i]);
-            }
-            frame.hide();
-        }
+        runThread.requestState(STOPPED);
     }
 
     /**
@@ -335,10 +316,7 @@ public final class KJASAppletStub
     */
     void initApplet()
     {
-        if( app != null ) {
-            app.init();
-        }
-        stateChange(INITIALIZED);
+        runThread.requestState(INITIALIZED);
    }
 
     /**
@@ -348,41 +326,7 @@ public final class KJASAppletStub
     */
     synchronized void destroyApplet()
     {
-        if (state >= DESTROYED || destroyThread != null)
-            return;
-        destroyThread = new Thread("applet destroy thread") {
-            public void run() {
-                if( runThread != null && runThread.isAlive() ) {
-                    Main.debug( "runThread is active when stub is dying" );
-                    tryToStopThread(runThread);
-                    runThread = null;
-                    panel.stopAnimation();
-                    loader.removeStatusListener(panel);
-                }
-                if( app != null ) {
-                    synchronized (app) {
-                        if (active) {
-                            try {
-                                stopApplet();
-                            } catch (Exception e) {
-                            }
-                        }
-                        try {
-                            app.destroy();
-                        } catch (Exception e) {
-                        }
-                    }
-                    frame.dispose();
-                    app = null;
-                    stateChange(DESTROYED);
-                }
-
-                loader = null;
-                context = null;
-                destroyThread = null;
-            }
-        };
-        destroyThread.start();
+        runThread.requestState(DESTROYED);
     }
 
     static void waitForDestroyThreads()
@@ -392,8 +336,9 @@ public final class KJASAppletStub
         for (int i = 0; i < len; i++) {
             try {
                 if (ts[i].getName() != null && 
-                        ts[i].getName().equals("applet destroy thread")) {
+                        ts[i].getName().startsWith("KJAS-AppletStub-")) {
                     try {
+                        ((RunThread) ts[i]).requestState(TERMINATE);
                         ts[i].join(10000);
                     } catch (InterruptedException ie) {}
                 }


More information about the kfm-devel mailing list