[Jnode-svn-commits] SF.net SVN: jnode:[4678] trunk

[<cr...@us...>]

Revision: 4678
          http://jnode.svn.sourceforge.net/jnode/?rev=4678&view=rev
Author:   crawley
Date:     2008-11-08 11:16:06 +0000 (Sat, 08 Nov 2008)

Log Message:
-----------
Mostly FIXME etc comments + a minor tweak to IsolateThreadImpl.stop ...
which doesn't seem to be getting called at the moment anyway.

Modified Paths:
--------------
    trunk/core/src/core/org/jnode/vm/isolate/VmIsolate.java
    trunk/shell/src/shell/org/jnode/shell/AsyncCommandInvoker.java
    trunk/shell/src/shell/org/jnode/shell/CommandThreadImpl.java
    trunk/shell/src/shell/org/jnode/shell/isolate/IsolateCommandThreadImpl.java

Modified: trunk/core/src/core/org/jnode/vm/isolate/VmIsolate.java
===================================================================
--- trunk/core/src/core/org/jnode/vm/isolate/VmIsolate.java	2008-11-08 07:59:17 UTC (rev 4677)
+++ trunk/core/src/core/org/jnode/vm/isolate/VmIsolate.java	2008-11-08 11:16:06 UTC (rev 4678)
@@ -422,10 +422,17 @@
     }
 
     private void stopAllThreads() {
+        // FIXME - this is probably unsafe because any of the threads being killed could
+        // be in the middle of updating a critical system data structure.  I'm also 
+        // unsure of the order in which we are killing the threads here.  It might be
+        // better to kill the isolate's main thread first to give it the chance to
+        // do a graceful shutdown.  (Stephen Crawley - 2008-11-08)
         int ac = threadGroup.activeCount();
         if (ac > 0) {
             Thread[] ta = new Thread[ac];
             int rc = threadGroup.enumerate(ta);
+            // FIXME - notwithstanding the above comments, is the 'current' thread the
+            // same one as the isolate's main thread?  (Stephen Crawley - 2008-11-08)
             Thread current = Thread.currentThread();
             boolean found = false;
             for (int i = 0; i < rc; i++) {
@@ -442,7 +449,7 @@
                 doExit();
             }
         } else {
-            //todo analyze this case      
+            // TODO - analyze this case      
             doExit();
         }
     }
@@ -456,7 +463,7 @@
         //on this isolate may call this method
         testIsolate(currentIsolate().isolate);
 
-        //todo handle demon threads
+        // FIXME - handle demon threads
         if (threadGroup.activeCount() > 0 || threadGroup.activeGroupCount() > 0)
             return;
 
@@ -476,7 +483,7 @@
         //on this isolate may call this method
         testIsolate(currentIsolate().isolate);
 
-        //todo handle demon threads
+        // FIXME - handle demon threads
         if (threadGroup.activeCount() > 0 || threadGroup.activeGroupCount() > 0)
             return;
 
@@ -773,7 +780,7 @@
             });
 
             //create the appcontext for this isolate
-            //todo improve this
+            // TODO - improve this
             Class.forName("sun.awt.SunToolkit").getMethod("createNewAppContext").invoke(null);
 
             // Update the state of this isolate.

Modified: trunk/shell/src/shell/org/jnode/shell/AsyncCommandInvoker.java
===================================================================
--- trunk/shell/src/shell/org/jnode/shell/AsyncCommandInvoker.java	2008-11-08 07:59:17 UTC (rev 4677)
+++ trunk/shell/src/shell/org/jnode/shell/AsyncCommandInvoker.java	2008-11-08 11:16:06 UTC (rev 4678)
@@ -214,7 +214,6 @@
         }
     }
 
-    @SuppressWarnings("deprecation")
     private void doCtrlC() {
         if (threadProcess != null && threadProcess.isAlive()
                 && blockingThread != null && blockingThread.isAlive()) {

Modified: trunk/shell/src/shell/org/jnode/shell/CommandThreadImpl.java
===================================================================
--- trunk/shell/src/shell/org/jnode/shell/CommandThreadImpl.java	2008-11-08 07:59:17 UTC (rev 4677)
+++ trunk/shell/src/shell/org/jnode/shell/CommandThreadImpl.java	2008-11-08 11:16:06 UTC (rev 4678)
@@ -93,6 +93,9 @@
     @SuppressWarnings("deprecation")
     @Override
     public void stop(ThreadDeath threadDeath) {
+        // FIXME - This is unsafe because the thread being killed could be in the
+        // middle of updating some critical system data structure.  We should
+        // probably throw an exception.
         super.stop(threadDeath);
     }
 

Modified: trunk/shell/src/shell/org/jnode/shell/isolate/IsolateCommandThreadImpl.java
===================================================================
--- trunk/shell/src/shell/org/jnode/shell/isolate/IsolateCommandThreadImpl.java	2008-11-08 07:59:17 UTC (rev 4677)
+++ trunk/shell/src/shell/org/jnode/shell/isolate/IsolateCommandThreadImpl.java	2008-11-08 11:16:06 UTC (rev 4678)
@@ -143,9 +143,7 @@
 
     @Override
     public void stop(ThreadDeath threadDeath) {
-        // FIXME - not sure what status argument should be ... but this is moot
-        // at the moment because VmIsolate.halt does nothing with it anyway.
-        isolate.halt(0 /* FIXME */);
+        isolate.halt(-1);
     }
     
     public void waitFor() throws ShellInvocationException {


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.