You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ta...@apache.org on 2016/11/03 11:03:54 UTC

svn commit: r1767866 - in /ofbiz/trunk/framework: base/src/main/java/org/apache/ofbiz/base/container/ start/src/main/java/org/apache/ofbiz/base/start/

Author: taher
Date: Thu Nov  3 11:03:54 2016
New Revision: 1767866

URL: http://svn.apache.org/viewvc?rev=1767866&view=rev
Log:
Reverted: partially reverted r1765127 to fix classpath issue
(OFBIZ-8337)

The above mentioned revision introduced a regression in which starting
OFBiz in debug mode triggers classpath exceptions due to not finding
certain jars. Upon investigation we realized that the new logic
instantiates the loader class before the custom ClassLoader is
instantiated. The fix would take a bit more time, hence reverting now
to quickly resolve the issue and make the code base ready for the next
release. The revert essentially re-introduce a List<StartupLoader> data
set instead of a single StartupLoader.

This is a minimal revert (less than 400 lines) instead of a full revert
going above 2000 lines because we isolated only the parts that trigger the
regression mentioned above. However, we should ideally revisit this code
and clean it up again with careful attention to the class loading sequence.

Modified:
    ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties

Modified: ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java?rev=1767866&r1=1767865&r2=1767866&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java (original)
+++ ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java Thu Nov  3 11:03:54 2016
@@ -22,6 +22,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Collectors;
 
 import org.apache.ofbiz.base.component.ComponentConfig;
@@ -56,7 +57,12 @@ public class ContainerLoader implements
     public synchronized void load(Config config, List<StartupCommand> ofbizCommands) throws StartupException {
 
         // loaders defined in startup (e.g. main, test, load-data, etc ...)
-        List<String> loaders = StringUtil.split((String) config.loader.get("profiles"), ",");
+        List<String> loaders = null;
+        for (Map<String,String> loaderMap: config.loaders) {
+            if (module.equals(loaderMap.get("class"))) {
+                loaders = StringUtil.split((String)loaderMap.get("profiles"), ",");
+            }
+        }
 
         // load containers defined in ofbiz-containers.xml
         Debug.logInfo("[Startup] Loading containers...", module);

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java?rev=1767866&r1=1767865&r2=1767866&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java Thu Nov  3 11:03:54 2016
@@ -24,6 +24,7 @@ import java.io.InputStreamReader;
 import java.io.PrintWriter;
 import java.net.ServerSocket;
 import java.net.Socket;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.ofbiz.base.start.Start.ServerState;
@@ -43,11 +44,11 @@ final class AdminServer extends Thread {
     }
 
     private ServerSocket serverSocket = null;
-    private StartupLoader loader = null;
+    private List<StartupLoader> loaders = null;
     private AtomicReference<ServerState> serverState = null;
     private Config config = null;
 
-    AdminServer(StartupLoader loader, AtomicReference<ServerState> serverState, Config config) throws StartupException {
+    AdminServer(List<StartupLoader> loaders, AtomicReference<ServerState> serverState, Config config) throws StartupException {
         super("OFBiz-AdminServer");
         try {
             this.serverSocket = new ServerSocket(config.adminPort, 1, config.adminAddress);
@@ -55,7 +56,7 @@ final class AdminServer extends Thread {
             throw new StartupException("Couldn't create server socket(" + config.adminAddress + ":" + config.adminPort + ")", e);
         }
         setDaemon(false);
-        this.loader = loader;
+        this.loaders = loaders;
         this.serverState = serverState;
         this.config = config;
     }
@@ -70,7 +71,7 @@ final class AdminServer extends Thread {
                         + clientSocket.getInetAddress() + " : "
                         + clientSocket.getPort());
 
-                processClientRequest(clientSocket, loader, serverState);
+                processClientRequest(clientSocket, loaders, serverState);
 
             } catch (IOException e) {
                 e.printStackTrace();
@@ -78,7 +79,7 @@ final class AdminServer extends Thread {
         }
     }
 
-    private void processClientRequest(Socket client, StartupLoader loader, AtomicReference<ServerState> serverState) throws IOException {
+    private void processClientRequest(Socket client, List<StartupLoader> loaders, AtomicReference<ServerState> serverState) throws IOException {
 
         try (BufferedReader reader = new BufferedReader(new InputStreamReader(client.getInputStream()));
                 PrintWriter writer = new PrintWriter(client.getOutputStream(), true)) {
@@ -94,7 +95,7 @@ final class AdminServer extends Thread {
             // if the client request is shutdown, execute shutdown sequence
             if(clientCommand.equals(OfbizSocketCommand.SHUTDOWN)) {
                 writer.flush();
-                StartupControlPanel.stop(loader, serverState, this);
+                StartupControlPanel.stop(loaders, serverState, this);
             }
         }
     }

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java?rev=1767866&r1=1767865&r2=1767866&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java Thu Nov  3 11:03:54 2016
@@ -25,6 +25,8 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -46,7 +48,7 @@ public final class Config {
     public final int portOffset;
     public final int adminPort;
     public final String containerConfig;
-    public final Map<String, String> loader;
+    public final List<Map<String, String>> loaders;
     public final String logDir;
     public final boolean shutdownAfterLoad;
     public final boolean useShutdownHook;
@@ -70,7 +72,7 @@ public final class Config {
         portOffset = getPortOffsetValue(ofbizCommands);
         adminPort = getAdminPort(props, portOffset);
         containerConfig = getAbsolutePath(props, "ofbiz.container.config", "framework/base/config/ofbiz-containers.xml", ofbizHome);
-        loader = getLoader(props);
+        loaders = getLoaders(props);
         logDir = getAbsolutePath(props, "ofbiz.log.dir", "runtime/logs", ofbizHome);
         shutdownAfterLoad = isShutdownAfterLoad(props);
         useShutdownHook = isUseShutdownHook(props);
@@ -232,11 +234,25 @@ public final class Config {
         return calculatedAdminPort;
     }
 
-    private Map<String, String> getLoader(Properties props) {
-        Map<String, String> loader = new HashMap<String, String>();
-        loader.put("class", props.getProperty("ofbiz.start.loader"));
-        loader.put("profiles", props.getProperty("ofbiz.start.loader.loaders"));
-        return loader;
+    private List<Map<String, String>> getLoaders(Properties props) {
+        ArrayList<Map<String, String>> loadersTmp = new ArrayList<Map<String, String>>();
+        int currentPosition = 1;
+        Map<String, String> loader = null;
+        while (true) {
+            loader = new HashMap<String, String>();
+            String loaderClass = props.getProperty("ofbiz.start.loader" + currentPosition);
+
+            if (loaderClass == null || loaderClass.length() == 0) {
+                break;
+            } else {
+                loader.put("class", loaderClass);
+                loader.put("profiles", props.getProperty("ofbiz.start.loader" + currentPosition + ".loaders"));
+                loadersTmp.add(Collections.unmodifiableMap(loader));
+                currentPosition++;
+            }
+        }
+        loadersTmp.trimToSize();
+        return Collections.unmodifiableList(loadersTmp);
     }
 
     private String getOfbizHome(Properties props) {

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java?rev=1767866&r1=1767865&r2=1767866&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java Thu Nov  3 11:03:54 2016
@@ -21,7 +21,9 @@ package org.apache.ofbiz.base.start;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.ofbiz.base.start.Start.ServerState;
@@ -59,15 +61,15 @@ final class StartupControlPanel {
             AtomicReference<ServerState> serverState,
             List<StartupCommand> ofbizCommands) throws StartupException {
 
-        StartupLoader loader = instantiateStartupLoader(config, Thread.currentThread().getContextClassLoader());
-        Thread adminServer = createAdminServer(config, serverState, loader);
+        List<StartupLoader> loaders = new ArrayList<StartupLoader>();
+        Thread adminServer = createAdminServer(config, serverState, loaders);
         Classpath classPath = createClassPath(config);
+        NativeLibClassLoader classLoader = createAndSetContextClassLoader(config, classPath);
 
-        createAndSetContextClassLoader(config, classPath);
         createLogDirectoryIfMissing(config);
-        createRuntimeShutdownHook(config, loader, serverState);
-        executeStartupLoadSequence(config, loader, ofbizCommands, serverState);
-        executeShutdownAfterLoadIfConfigured(config, loader, serverState, adminServer);
+        createRuntimeShutdownHook(config, loaders, serverState);
+        loadStartupLoaders(config, loaders, ofbizCommands, serverState, classLoader);
+        executeShutdownAfterLoadIfConfigured(config, loaders, serverState, adminServer);
     }
 
     /**
@@ -77,8 +79,8 @@ final class StartupControlPanel {
      * - Manually if requested by the client AdminClient
      * - Automatically if Config.shutdownAfterLoad is set to true
      */
-    static void stop(StartupLoader loader, AtomicReference<ServerState> serverState, Thread adminServer) {
-        shutdownServer(loader, serverState, adminServer);
+    static void stop(List<StartupLoader> loaders, AtomicReference<ServerState> serverState, Thread adminServer) {
+        shutdownServer(loaders, serverState, adminServer);
         System.exit(0);
     }
 
@@ -102,7 +104,7 @@ final class StartupControlPanel {
         System.exit(1);
     }
 
-    private static void shutdownServer(StartupLoader loader, AtomicReference<ServerState> serverState, Thread adminServer) {
+    private static void shutdownServer(List<StartupLoader> loaders, AtomicReference<ServerState> serverState, Thread adminServer) {
         ServerState currentState;
         do {
             currentState = serverState.get();
@@ -112,10 +114,16 @@ final class StartupControlPanel {
         } while (!serverState.compareAndSet(currentState, ServerState.STOPPING));
         // The current thread was the one that successfully changed the state;
         // continue with further processing.
-        try {
-            loader.unload();
-        } catch (Exception e) {
-            e.printStackTrace();
+        synchronized (loaders) {
+            // Unload in reverse order
+            for (int i = loaders.size(); i > 0; i--) {
+                StartupLoader loader = loaders.get(i - 1);
+                try {
+                    loader.unload();
+                } catch (Exception e) {
+                    e.printStackTrace();
+                }
+            }
         }
         if (adminServer != null && adminServer.isAlive()) {
             adminServer.interrupt();
@@ -136,26 +144,14 @@ final class StartupControlPanel {
         }
     }
 
-    private static StartupLoader instantiateStartupLoader(Config config, ClassLoader classLoader) throws StartupException {
-        StartupLoader loader;
-        try {
-            String className = config.loader.get("class");
-            Class<?> loaderClass = classLoader.loadClass(className);
-            loader = (StartupLoader) loaderClass.newInstance();
-        } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) {
-            throw new StartupException("Could not initiate a StartupLoader", e);
-        }
-        return loader;
-    }
-
     private static Thread createAdminServer(
             Config config,
             AtomicReference<ServerState> serverState,
-            StartupLoader loader) throws StartupException {
+            List<StartupLoader> loaders) throws StartupException {
 
         Thread adminServer = null;
         if (config.adminPort > 0) {
-            adminServer = new AdminServer(loader, serverState, config);
+            adminServer = new AdminServer(loaders, serverState, config);
             adminServer.start();
         } else {
             System.out.println("Admin socket not configured; set to port 0");
@@ -210,14 +206,14 @@ final class StartupControlPanel {
 
     private static void createRuntimeShutdownHook(
             Config config,
-            StartupLoader loader,
+            List<StartupLoader> loaders,
             AtomicReference<ServerState> serverState) {
 
         if (config.useShutdownHook) {
             Runtime.getRuntime().addShutdownHook(new Thread() {
                 @Override
                 public void run() {
-                    shutdownServer(loader, serverState, this);
+                    shutdownServer(loaders, serverState, this);
                 }
             });
         } else {
@@ -225,27 +221,46 @@ final class StartupControlPanel {
         }
     }
 
-    private static void executeStartupLoadSequence(Config config, 
-            StartupLoader loader,
+    private static void loadStartupLoaders(Config config, 
+            List<StartupLoader> loaders,
             List<StartupCommand> ofbizCommands,
-            AtomicReference<ServerState> serverState) throws StartupException {
+            AtomicReference<ServerState> serverState,
+            NativeLibClassLoader classloader) throws StartupException {
 
-        if (serverState.get() != ServerState.STOPPING) {
-            loader.load(config, ofbizCommands);
+        synchronized (loaders) {
+            for (Map<String, String> loaderMap : config.loaders) {
+                if (serverState.get() == ServerState.STOPPING) {
+                    return;
+                }
+                try {
+                    String loaderClassName = loaderMap.get("class");
+                    Class<?> loaderClass = classloader.loadClass(loaderClassName);
+                    StartupLoader loader = (StartupLoader) loaderClass.newInstance();
+                    loaders.add(loader); // add before loading, so unload can occur if error during loading
+                    loader.load(config, ofbizCommands);
+                } catch (ReflectiveOperationException e) {
+                    throw new StartupException(e.getMessage(), e);
+                }
+            }
         }
-        if(!serverState.compareAndSet(ServerState.STARTING, ServerState.RUNNING)) {
-            throw new StartupException("Error during start");
+        StringBuilder sb = new StringBuilder();
+        for (String path : classloader.getNativeLibPaths()) {
+            if (sb.length() > 0) {
+                sb.append(File.pathSeparator);
+            }
+            sb.append(path);
         }
+        System.setProperty("java.library.path", sb.toString());
     }
 
     private static void executeShutdownAfterLoadIfConfigured(
             Config config,
-            StartupLoader loader,
+            List<StartupLoader> loaders,
             AtomicReference<ServerState> serverState,
             Thread adminServer) {
 
         if (config.shutdownAfterLoad) {
-            stop(loader, serverState, adminServer);
+            stop(loaders, serverState, adminServer);
         }
     }
 }

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties?rev=1767866&r1=1767865&r2=1767866&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties Thu Nov  3 11:03:54 2016
@@ -30,8 +30,8 @@ ofbiz.start.classpath.addComponent=frame
 #ofbiz.container.config=framework/base/config/ofbiz-containers.xml
 
 # --- StartupLoader implementations to load (in order)
-ofbiz.start.loader=org.apache.ofbiz.base.container.ContainerLoader
-ofbiz.start.loader.loaders=load-data
+ofbiz.start.loader1=org.apache.ofbiz.base.container.ContainerLoader
+ofbiz.start.loader1.loaders=load-data
 
 # -- Enable the shutdown hook
 #ofbiz.enable.hook=false

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties?rev=1767866&r1=1767865&r2=1767866&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties Thu Nov  3 11:03:54 2016
@@ -30,8 +30,8 @@ ofbiz.start.classpath.addComponent=frame
 #ofbiz.container.config=framework/base/config/ofbiz-containers.xml
 
 # --- StartupLoader implementations to load (in order)
-ofbiz.start.loader=org.apache.ofbiz.base.container.ContainerLoader
-ofbiz.start.loader.loaders=rmi
+ofbiz.start.loader1=org.apache.ofbiz.base.container.ContainerLoader
+ofbiz.start.loader1.loaders=rmi
 
 # -- Enable the shutdown hook
 #ofbiz.enable.hook=true

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties?rev=1767866&r1=1767865&r2=1767866&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties Thu Nov  3 11:03:54 2016
@@ -38,11 +38,11 @@ ofbiz.admin.key=so3du5kasd5dn
 #ofbiz.container.config=framework/base/config/ofbiz-containers.xml
 
 # --- StartupLoader implementations to load (in order)
-ofbiz.start.loader=org.apache.ofbiz.base.container.ContainerLoader
+ofbiz.start.loader1=org.apache.ofbiz.base.container.ContainerLoader
 # Because of the danger of Java deserialization when using RMI, the RMI component has been disabled in the default configuration of OFBiz.
 # If you need RMI you just need to uncomment those places - See OFBIZ-6942 for details -->
 #ofbiz.start.loader1.loaders=main,rmi
-ofbiz.start.loader.loaders=main
+ofbiz.start.loader1.loaders=main
  
 # -- Enable the shutdown hook
 #ofbiz.enable.hook=true

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties?rev=1767866&r1=1767865&r2=1767866&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties Thu Nov  3 11:03:54 2016
@@ -30,8 +30,8 @@ ofbiz.start.classpath.addComponent=frame
 #ofbiz.container.config=framework/base/config/ofbiz-containers.xml
 
 # --- StartupLoader implementations to load (in order)
-ofbiz.start.loader=org.apache.ofbiz.base.container.ContainerLoader
-ofbiz.start.loader.loaders=test
+ofbiz.start.loader1=org.apache.ofbiz.base.container.ContainerLoader
+ofbiz.start.loader1.loaders=test
 
 # -- Enable the shutdown hook
 #ofbiz.enable.hook=true