Changeset 29947 for main

Timestamp:

2015-06-05T20:58:20+12:00 (9 years ago)

Author:

ak19

Message:

First of two commits to fix rebuilding a collection using the online editor on Windows. Resolves the file lock problem. The java code works with changes in activate.pl. activate.pl is passed a flag so that it no longer de-activates and re-activates the collection itself, but just concerns itself with moving building to index. The GS2Construct java code now de-activates the collection prior to calling activate.pl and then re-activates it afterward. In the way it was done before, activate.pl used to handle de- and re-activating the collection. But when it was launched from the server java code, the java VM would exit having left a copy of the file handles to the perl process when forking the process for activate.pl. The perl code could not move building to index since the file handles had locks (6 of them) on the index/text/collection.gdb. Changes have been made to GS2PerlConstructor too, so that it more cleanly closes all the pipes of a process, that the process itself may thereby exit cleanly. Not yet able to move this properly into its own classes since the StreamGobbler classes in GLI are not quite suited but were customised for FormatConverter.

Location:

main/trunk/greenstone3/src/java/org/greenstone/gsdl3

Files:

• build/GS2PerlConstructor.java (modified) (10 diffs)
• service/GS2Construct.java (modified) (2 diffs)

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/GS2PerlConstructor.java

@@ -13 +13 @@
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
+import java.io.Closeable;
 import java.io.FileWriter;
 import java.io.InputStreamReader;
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Vector;
@@ -278 +280 @@
         command.add(GSFile.collectDir(this.site_home));
         command.add("-removeold"); // saves some seconds processing time when this flag's added in explicitly
+        command.add("-skipactivation"); // gsdl3/util/GS2Construct does the activation and reactivation
         command.addAll(extractParameters(this.process_params));
         command.add(this.collection_name);
@@ -365 +368 @@
     return runPerlCommand(command, null, null);
     }
-
-    protected boolean runPerlCommand(String[] command, String[] envvars, File dir)
-    {
+    
+    protected boolean runPerlCommand(String[] command, String[] envvars, File dir)
+    {
+        boolean success = true;
+    
         int sepIndex = this.gsdl3home.lastIndexOf(File.separator);
         String srcHome = this.gsdl3home.substring(0, sepIndex);
@@ -397 +402 @@
 
         sendMessage(new ConstructionEvent(this, GSStatus.INFO, "command = " + command_str));
+        Process prcs = null;
+        BufferedReader ebr = null;
+        BufferedReader stdinbr = null;
         try
         {
             Runtime rt = Runtime.getRuntime();
             sendProcessBegun(new ConstructionEvent(this, GSStatus.ACCEPTED, "starting"));
-            Process prcs = (dir == null) 
+            prcs = (dir == null) 
                 ? rt.exec(command, args.toArray(new String[args.size()]))
                 : rt.exec(command, args.toArray(new String[args.size()]), dir);
@@ -407 +415 @@
             InputStreamReader eisr = new InputStreamReader(prcs.getErrorStream());
             InputStreamReader stdinisr = new InputStreamReader(prcs.getInputStream());
-            BufferedReader ebr = new BufferedReader(eisr);
-            BufferedReader stdinbr = new BufferedReader(stdinisr);
+            ebr = new BufferedReader(eisr);
+            stdinbr = new BufferedReader(stdinisr);
             // Captures the std err of a program and pipes it into
             // std in of java
@@ -440 +448 @@
                 }
             }
-            bw.close();
-
+            closeResource(bw); 
+            
             if (!this.cancel)
             {
@@ -448 +456 @@
 
                 if (prcs.exitValue() == 0)
-                {
+                {   
                     //status = OK;
                     sendProcessStatus(new ConstructionEvent(this, GSStatus.CONTINUING, "Success"));
-
+                    
+                    success = true;
                 }
                 else
@@ -458 +467 @@
                     sendProcessStatus(new ConstructionEvent(this, GSStatus.ERROR, "Failure"));
 
-                    return false;
+                    //return false;
+                    success = false;
 
                 }
@@ -466 +476 @@
                 // I need to somehow kill the child process. Unfortunately Thread.stop() and Process.destroy() both fail to do this. But now, thankx to the magic of Michaels 'close the stream suggestion', it works fine.
                 sendProcessStatus(new ConstructionEvent(this, GSStatus.HALTED, "killing the process"));
-                prcs.getOutputStream().close();
-                prcs.destroy();
-                //status = ERROR;
-                return false;
+                //prcs.getOutputStream().close();
+                //prcs.destroy();
+                ////status = ERROR;
+                
+                //return false;
+                success = false;
             }
         }
@@ -476 +488 @@
             e.printStackTrace();
             sendProcessStatus(new ConstructionEvent(this, GSStatus.ERROR, "Exception occurred " + e.toString()));
-        }
-
-        // we're done, but we dont send a process complete message here cos there ight be stuff to do after this has finished.
-        return true;
-
-    }
-
+        } finally { 
+            // http://steveliles.github.io/invoking_processes_from_java.html
+            // http://www.javaworld.com/article/2071275/core-java/when-runtime-exec---won-t.html?page=2
+            // http://mark.koli.ch/leaky-pipes-remember-to-close-your-streams-when-using-javas-runtimegetruntimeexec
+        
+             if( prcs != null ) {
+                closeResource(prcs.getErrorStream());
+                closeResource(prcs.getOutputStream());
+                closeResource(prcs.getInputStream());
+                prcs.destroy();
+            }
+        
+            closeResource(ebr);
+            closeResource(stdinbr);
+        }
+
+        // we're done, but we don't send a process complete message here cos there might be stuff to do after this has finished.
+        //return true;      
+        return success;
+    }
+    
+    public static void closeResource(Closeable resourceHandle) {
+        try {
+            if(resourceHandle != null) {
+                resourceHandle.close();
+                resourceHandle = null;
+            }
+        } catch(Exception e) {
+            System.err.println("Exception closing resource: " + e.getMessage());
+            e.printStackTrace();
+        }
+    }
 }

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/GS2Construct.java

@@ -330 +330 @@
         String lang = request.getAttribute(GSXML.LANG_ATT);
 
-        Element response = runCommand(request, GS2PerlConstructor.ACTIVATE);
+        UserContext userContext = new UserContext(request);
+        String request_type = request.getAttribute(GSXML.TYPE_ATT);
+        
+        // now we de-activate the collection before running activate.pl, and then re-activate at end 
+        // So activate.pl only does the moving, no activation. This way will prevent java from launching 
+        // perl, exiting and then leaving dangling file handles (on index/text/col.gdb) in perl.
+        if (!request_type.equals(GSXML.REQUEST_TYPE_STATUS)) {          
+            systemRequest("delete", coll_name, null, userContext); // deactivate collection
+        }   
+
+        Element response = runCommand(request, GS2PerlConstructor.ACTIVATE); // if request is for STATUS, then this won't run activate.pl
+
         Element status = (Element) GSXML.getChildByTagName(response, GSXML.STATUS_ELEM);
-
-        String request_type = request.getAttribute(GSXML.TYPE_ATT);
-        if (request_type.equals(GSXML.REQUEST_TYPE_STATUS))
-        {
-            return response;
-        }
-
-        UserContext userContext = new UserContext(request);
-        systemRequest("delete", coll_name, null, userContext);
 
         // check for finished
@@ -384 +386 @@
         status.appendChild(t);
         // once have got here, we assume
-        // the first bit proceeded successfully, now reload the collection
+        // the first bit proceeded successfully, now reload the collection (sends a collection reactivation request)
         systemRequest("reload", coll_name, status, userContext); // this will append more messages to the status, and overwrite the error code att
         return response;