From aa67827feba9c57fc5250580f7c07b709ec5a289 Mon Sep 17 00:00:00 2001 From: Mathieu Lirzin Date: Sat, 13 Jul 2019 22:27:04 +0000 Subject: [PATCH] =?UTF-8?q?Improved:=20Move=20=E2=80=98AdminServer?= =?UTF-8?q?=E2=80=99=20inside=20a=20container=20(OFBIZ-11136)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ‘AdminServer’ provides a portable way to manage life-cycle of the OFBiz process remotely by allowing administrator to check its running status or shutting it down. Previously the ‘AdminServer’ class was a special thread opening a socket and launched at startup. However since this class is about managing some run-time resources with a life-cycle, it matches perfectly the container abstraction. A benefit of making ‘AdminServer’ a container is that the startup process is now simpler and more uniform. Administrators can now prevent remote shutdown of OFBiz for security reasons by removing the container declaration. Additionally They can delegate the process management job to the init process (PID 0) of the hosting system like Systemd [1] by replacing this container with another one. [1] https://www.freedesktop.org/software/systemd/man/systemd-notify.html git-svn-id: https://svn.apache.org/repos/asf/ofbiz/ofbiz-framework/trunk@1863024 13f79535-47bb-0310-9956-ffa450edef68 --- framework/base/ofbiz-component.xml | 2 + .../base/container/AdminServerContainer.java} | 98 +++++++++++-------- .../apache/ofbiz/base/start/AdminClient.java | 2 +- .../org/apache/ofbiz/base/start/Start.java | 10 +- .../ofbiz/base/start/StartupControlPanel.java | 33 +------ 5 files changed, 76 insertions(+), 69 deletions(-) rename framework/{start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java => base/src/main/java/org/apache/ofbiz/base/container/AdminServerContainer.java} (62%) diff --git a/framework/base/ofbiz-component.xml b/framework/base/ofbiz-component.xml index 37dfb717fc3..9ccd7889969 100644 --- a/framework/base/ofbiz-component.xml +++ b/framework/base/ofbiz-component.xml @@ -32,5 +32,7 @@ under the License. + + diff --git a/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java b/framework/base/src/main/java/org/apache/ofbiz/base/container/AdminServerContainer.java similarity index 62% rename from framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java rename to framework/base/src/main/java/org/apache/ofbiz/base/container/AdminServerContainer.java index 9598b790911..3f6e3f2be25 100644 --- a/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java +++ b/framework/base/src/main/java/org/apache/ofbiz/base/container/AdminServerContainer.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. *******************************************************************************/ -package org.apache.ofbiz.base.start; +package org.apache.ofbiz.base.container; import java.io.BufferedReader; import java.io.IOException; @@ -26,10 +26,12 @@ import java.net.ServerSocket; import java.net.Socket; import java.nio.charset.StandardCharsets; -import java.util.concurrent.atomic.AtomicReference; +import java.util.List; -import org.apache.ofbiz.base.container.ContainerLoader; +import org.apache.ofbiz.base.start.Config; +import org.apache.ofbiz.base.start.Start; import org.apache.ofbiz.base.start.Start.ServerState; +import org.apache.ofbiz.base.start.StartupCommand; import org.apache.ofbiz.base.util.UtilValidate; /** @@ -37,60 +39,77 @@ * OFBiz instance after it has started and send commands to that instance * such as inquiring on server status or requesting system shutdown */ -final class AdminServer extends Thread { - +public final class AdminServerContainer implements Container { /** * Commands communicated between AdminClient and AdminServer */ - enum OfbizSocketCommand { + public enum OfbizSocketCommand { SHUTDOWN, STATUS, FAIL } - private ServerSocket serverSocket = null; - private ContainerLoader loader; - private AtomicReference serverState = null; - private Config config = null; + private String name; + private Thread serverThread; + private ServerSocket serverSocket; + private Config cfg = Start.getInstance().getConfig(); - AdminServer(ContainerLoader loader, AtomicReference serverState, Config config) throws StartupException { - super("OFBiz-AdminServer"); + @Override + public void init(List ofbizCommands, String name, String configFile) throws ContainerException { + this.name = name; try { - this.serverSocket = new ServerSocket(config.adminPort, 1, config.adminAddress); + serverSocket = new ServerSocket(cfg.adminPort, 1, cfg.adminAddress); } catch (IOException e) { - throw new StartupException("Couldn't create server socket(" + config.adminAddress + ":" + config.adminPort + ")", e); + String msg = "Couldn't create server socket(" + cfg.adminAddress + ":" + cfg.adminPort + ")"; + throw new ContainerException(msg, e); } - setDaemon(false); - this.loader = loader; - this.serverState = serverState; - this.config = config; + + if (cfg.adminPort > 0) { + serverThread = new Thread(this::run, "OFBiz-AdminServer"); + } else { + serverThread = new Thread("OFBiz-AdminServer"); // Dummy thread + System.out.println("Admin socket not configured; set to port 0"); + } + serverThread.setDaemon(false); } - @Override - public void run() { - System.out.println("Admin socket configured on - " + config.adminAddress + ":" + config.adminPort); + // Listens for administration commands. + private void run() { + System.out.println("Admin socket configured on - " + cfg.adminAddress + ":" + cfg.adminPort); while (!Thread.interrupted()) { - try (Socket clientSocket = serverSocket.accept()){ - - System.out.println("Received connection from - " - + clientSocket.getInetAddress() + " : " - + clientSocket.getPort()); - - processClientRequest(clientSocket, loader, serverState); - + try (Socket client = serverSocket.accept()) { + System.out.println("Received connection from - " + client.getInetAddress() + " : " + client.getPort()); + processClientRequest(client); } catch (IOException e) { e.printStackTrace(); } } } - private void processClientRequest(Socket client, ContainerLoader loader, AtomicReference serverState) - throws IOException { + @Override + public boolean start() throws ContainerException { + serverThread.start(); + return true; + } + + @Override + public void stop() throws ContainerException { + if (serverThread.isAlive()) { + serverThread.interrupt(); + } + } + + @Override + public String getName() { + return name; + } + + private void processClientRequest(Socket client) throws IOException { try (BufferedReader reader = new BufferedReader(new InputStreamReader(client.getInputStream(), StandardCharsets.UTF_8)); - PrintWriter writer = new PrintWriter(new OutputStreamWriter(client.getOutputStream(), StandardCharsets.UTF_8), true)) { + PrintWriter writer = new PrintWriter(new OutputStreamWriter(client.getOutputStream(), StandardCharsets.UTF_8), true)) { // read client request and prepare response String clientRequest = reader.readLine(); OfbizSocketCommand clientCommand = determineClientCommand(clientRequest); - String serverResponse = prepareResponseToClient(clientCommand, serverState); + String serverResponse = prepareResponseToClient(clientCommand); // send response back to client writer.println(serverResponse); @@ -98,8 +117,7 @@ private void processClientRequest(Socket client, ContainerLoader loader, AtomicR // if the client request is shutdown, execute shutdown sequence if(clientCommand.equals(OfbizSocketCommand.SHUTDOWN)) { writer.flush(); - StartupControlPanel.shutdownServer(loader, serverState, this); - System.exit(0); + Start.getInstance().stop(); } } } @@ -119,22 +137,24 @@ private OfbizSocketCommand determineClientCommand(String request) { private boolean isValidRequest(String request) { return UtilValidate.isNotEmpty(request) && request.contains(":") - && request.substring(0, request.indexOf(':')).equals(config.adminKey) + && request.substring(0, request.indexOf(':')).equals(cfg.adminKey) && !request.substring(request.indexOf(':') + 1).isEmpty(); } - private static String prepareResponseToClient(OfbizSocketCommand control, AtomicReference serverState) { + + private static String prepareResponseToClient(OfbizSocketCommand control) { String response = null; + ServerState state = Start.getInstance().getCurrentState(); switch(control) { case SHUTDOWN: - if (serverState.get() == ServerState.STOPPING) { + if (state == ServerState.STOPPING) { response = "IN-PROGRESS"; } else { response = "OK"; } break; case STATUS: - response = serverState.get().toString(); + response = state.toString(); break; case FAIL: response = "FAIL"; diff --git a/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminClient.java b/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminClient.java index 7800e29c673..6050ef0d15e 100644 --- a/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminClient.java +++ b/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminClient.java @@ -27,7 +27,7 @@ import java.net.Socket; import java.nio.charset.StandardCharsets; -import org.apache.ofbiz.base.start.AdminServer.OfbizSocketCommand; +import org.apache.ofbiz.base.container.AdminServerContainer.OfbizSocketCommand; /** * The AdminClient communicates with a running OFBiz server instance diff --git a/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java b/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java index 87386be1e03..d223de7750c 100644 --- a/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java +++ b/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java @@ -24,6 +24,8 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; +import org.apache.ofbiz.base.container.ContainerLoader; + /** * OFBiz startup class. * @@ -44,6 +46,7 @@ public final class Start { private Config config = null; + private ContainerLoader loader = new ContainerLoader(); private final AtomicReference serverState = new AtomicReference<>(ServerState.STARTING); // Singleton, do not change @@ -84,7 +87,7 @@ public static void main(String[] args) { break; case START: try { - StartupControlPanel.start(instance.config, instance.serverState, ofbizCommands); + StartupControlPanel.start(instance.config, instance.serverState, ofbizCommands, instance.loader); } catch (StartupException e) { StartupControlPanel.fullyTerminateSystem(e); } @@ -113,6 +116,11 @@ public ServerState getCurrentState() { return serverState.get(); } + public void stop() { + StartupControlPanel.shutdownServer(loader, serverState); + System.exit(0); + } + /** * This enum contains the possible OFBiz server states. */ diff --git a/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java b/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java index 25aa327a53b..7049cea9855 100644 --- a/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java +++ b/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java @@ -58,13 +58,8 @@ static Config init(List ofbizCommands) { /** * Execute the startup sequence for OFBiz */ - static void start(Config config, - AtomicReference serverState, - List ofbizCommands) throws StartupException { - - ContainerLoader loader = new ContainerLoader(); - Thread adminServer = createAdminServer(config, serverState, loader); - + static void start(Config config, AtomicReference serverState, List ofbizCommands, + ContainerLoader loader) throws StartupException { createLogDirectoryIfMissing(config.logDir); if (config.useShutdownHook) { @@ -76,7 +71,7 @@ static void start(Config config, loadContainers(config, loader, ofbizCommands, serverState); if (config.shutdownAfterLoad) { - shutdownServer(loader, serverState, adminServer); + shutdownServer(loader, serverState); System.exit(0); } else { // Print startup message. @@ -110,7 +105,7 @@ static void fullyTerminateSystem(StartupException e) { System.exit(1); } - static void shutdownServer(ContainerLoader loader, AtomicReference serverState, Thread adminServer) { + static void shutdownServer(ContainerLoader loader, AtomicReference serverState) { ServerState currentState; do { currentState = serverState.get(); @@ -123,9 +118,6 @@ static void shutdownServer(ContainerLoader loader, AtomicReference } catch (Exception e) { e.printStackTrace(); } - if (adminServer != null && adminServer.isAlive()) { - adminServer.interrupt(); - } } private static void loadGlobalOfbizSystemProperties(String globalOfbizPropertiesFileName) throws StartupException { @@ -139,21 +131,6 @@ private static void loadGlobalOfbizSystemProperties(String globalOfbizProperties } } - private static Thread createAdminServer( - Config config, - AtomicReference serverState, - ContainerLoader loader) throws StartupException { - - Thread adminServer = null; - if (config.adminPort > 0) { - adminServer = new AdminServer(loader, serverState, config); - adminServer.start(); - } else { - System.out.println("Admin socket not configured; set to port 0"); - } - return adminServer; - } - private static void createLogDirectoryIfMissing(String logDirName) { File logDir = new File(logDirName); if (!logDir.exists()) { @@ -167,7 +144,7 @@ private static void createRuntimeShutdownHook(ContainerLoader loader, AtomicRefe Runtime.getRuntime().addShutdownHook(new Thread() { @Override public void run() { - shutdownServer(loader, serverState, this); + shutdownServer(loader, serverState); } }); }