Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ private File getPasswordFile() {
*/
private AsadminResult exec(final Integer timeout, final boolean detachedAndTerse, final String... args) {
final List<String> parameters = Arrays.asList(args);
LOG.log(TRACE, "exec(timeout={0}, detached={1}, args={2})", timeout, detachedAndTerse, parameters);
LOG.log(INFO, "exec(timeout={0}, detached={1}, args={2})", timeout, detachedAndTerse, parameters);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not seeing this when I had issues on GHA was very confusing.

final List<String> command = new ArrayList<>();
if (asadmin.getName().endsWith(".java")) {
command.add(JAVA_EXECUTABLE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,28 @@

package org.glassfish.enterprise.iiop.api;

import com.sun.logging.LogDomains;

import jakarta.annotation.PostConstruct;
import jakarta.annotation.PreDestroy;
import jakarta.ejb.Singleton;
import jakarta.inject.Inject;
import jakarta.inject.Provider;

import java.lang.System.Logger;
import java.nio.channels.SelectableChannel;
import java.rmi.Remote;
import java.util.Properties;
import java.util.logging.Level;
import java.util.logging.Logger;

import org.glassfish.api.admin.ProcessEnvironment;
import org.glassfish.api.event.EventListener;
import org.glassfish.api.event.Events;
import org.glassfish.api.naming.GlassfishNamingManager;
import org.glassfish.hk2.api.ServiceLocator;
import org.glassfish.internal.api.ORBLocator;
import org.jvnet.hk2.annotations.Service;
import org.omg.CORBA.ORB;
import org.omg.PortableInterceptor.ServerRequestInfo;

import static com.sun.logging.LogDomains.CORBA_LOGGER;
import static java.lang.System.Logger.Level.INFO;
import static org.glassfish.api.event.EventTypes.SERVER_SHUTDOWN;

/**
* This class exposes any orb/iiop functionality needed by modules in the app server.
Expand All @@ -51,7 +50,10 @@
@Singleton
public class GlassFishORBHelper implements ORBLocator {

private static final Logger LOG = LogDomains.getLogger(GlassFishORBHelper.class, CORBA_LOGGER, false);
private static final Logger LOG = System.getLogger(GlassFishORBHelper.class.getName());

@Inject
private Provider<Events> eventsProvider;

@Inject
private ServiceLocator services;
Expand All @@ -75,17 +77,35 @@ public class GlassFishORBHelper implements ORBLocator {

@PostConstruct
public void postConstruct() {
// WARN: Neither PreDestroy annotation nor interface worked!
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PreDestroy annotation doesn't work here.
PreDestrou interface doesn't work too.
This was here on 2024, in one change I removed it, I was wrong.

EventListener glassfishEventListener = event -> {
if (event.is(SERVER_SHUTDOWN)) {
onShutdown();
}
};
eventsProvider.get().register(glassfishEventListener);
orbFactory = services.getService(GlassFishORBFactory.class);
LOG.log(INFO, "GlassFishORBLocator created.");
}

@PreDestroy
public void onShutdown() {
private void onShutdown() {
// FIXME: getORB is able to create another, it should be refactored and simplified.
destroyed = true;
LOG.log(Level.CONFIG, "ORB Shutdown started");
if (orb != null) {
orb.destroy();
LOG.log(INFO, "ORB shutdown started");
if (this.orb != null) {
// First remove, then destroy.
// Still, threads already working with the instance will have it unstable.
final ORB destroyedOrb = orb;
orb = null;
// FIXME: com.sun.corba.ee.impl.transport.AcceptorImpl.getAcceptedSocket(AcceptorImpl.java:127)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#25804 will target this.

// can still be blocked in standalone thread, that would lead to its failure
// and cascade leading sockets open. Restart of the server could fail then.
try {
Thread.sleep(1000L);
} catch (InterruptedException e) {
// We don't want to interrupt here.
}
destroyedOrb.destroy();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@
import java.net.SocketException;
import java.nio.channels.ServerSocketChannel;
import java.nio.channels.SocketChannel;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Hashtable;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -89,7 +92,7 @@ public class IIOPSSLSocketFactory implements ORBSocketFactory {
* @todo provide an interface to the admin, so that whenever a iiop-listener
* is added / removed, we modify the hashtable,
*/
private final Map portToSSLInfo = new Hashtable();
private final Map<Integer, SSLInfo> portToSSLInfo = new Hashtable<>();
/* this is stored for the client side of SSL Connections.
* Note: There will be only 1 ctx for the client side, as we will reuse the
* ctx for all SSL connections
Expand Down Expand Up @@ -175,8 +178,7 @@ public IIOPSSLSocketFactory() {
}
}
} catch (Exception e) {
LOG.log(Level.SEVERE,"IIOPSSLSocketFactory initialization failed.", e);
throw new IllegalStateException(e);
throw new IllegalStateException("IIOPSSLSocketFactory initialization failed.", e);
}
}

Expand Down Expand Up @@ -251,25 +253,23 @@ public void setORB(ORB orb) {
*/
@Override
public ServerSocket createServerSocket(String type, InetSocketAddress inetSocketAddress) throws IOException {
if (LOG.isLoggable(Level.FINE)) {
LOG.log(Level.FINE, "Creating server socket for type =" + type
+ " inetSocketAddress =" + inetSocketAddress);
}
LOG.log(Level.INFO, "Creating server socket for type =" + type + " inetSocketAddress =" + inetSocketAddress);

if(type.equals(SSL_MUTUALAUTH) || type.equals(SSL) ||
type.equals(PERSISTENT_SSL)) {
if (type.equals(SSL_MUTUALAUTH) || type.equals(SSL) || type.equals(PERSISTENT_SSL)) {
return createSSLServerSocket(type, inetSocketAddress);
}
ServerSocket serverSocket = null;
final ServerSocket serverSocket;
if (orb.getORBData().acceptorSocketType().equals(SOCKETCHANNEL)) {
ServerSocketChannel serverSocketChannel = ServerSocketChannel.open();
serverSocket = serverSocketChannel.socket();
} else {
serverSocket = new ServerSocket();
}

serverSocket.bind(inetSocketAddress);
return serverSocket;
final Action<ServerSocket> action = () -> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Desperate action, I will recheck in #25804 if it helps anything. When GF + Corba will reliably close all open ports on shutdown, this should be removed.

serverSocket.bind(inetSocketAddress);
return serverSocket;
};
return repeat(action, Duration.ofSeconds(10L));
}

/**
Expand Down Expand Up @@ -339,7 +339,7 @@ private ServerSocket createSSLServerSocket(String type, InetSocketAddress inetSo
}
int port = inetSocketAddress.getPort();
Integer iport = Integer.valueOf(port);
SSLInfo sslInfo = (SSLInfo)portToSSLInfo.get(iport);
SSLInfo sslInfo = portToSSLInfo.get(iport);
if (sslInfo == null) {
throw new IOException("No SSL info found for port " + iport);
}
Expand All @@ -351,42 +351,21 @@ private ServerSocket createSSLServerSocket(String type, InetSocketAddress inetSo
String[] socketCiphers = ssf.getDefaultCipherSuites();
ciphers = mergeCiphers(socketCiphers, ssl3TlsCiphers, ssl2Ciphers);
}

String cs[] = null;

if (LOG.isLoggable(Level.FINE)) {
cs = ssf.getSupportedCipherSuites();
for (String element : cs) {
LOG.log(Level.FINE, "Cipher Suite: " + element);
}
LOG.log(Level.FINE, () -> "Supported cipher Suites: " + Arrays.toString(ssf.getSupportedCipherSuites()));
final Action<ServerSocket> action = () -> ssf.createServerSocket(port, BACKLOG, inetSocketAddress.getAddress());
final ServerSocket ss = repeat(action, Duration.ofSeconds(10L));
if (ciphers != null) {
((SSLServerSocket) ss).setEnabledCipherSuites(ciphers);
}
ServerSocket ss = null;
try {
// bugfix for 6349541
// specify the ip address to bind to, 50 is the default used
// by the ssf implementation when only the port is specified
ss = ssf.createServerSocket(port, BACKLOG, inetSocketAddress.getAddress());
if (ciphers != null) {
((SSLServerSocket) ss).setEnabledCipherSuites(ciphers);
}
} catch (IOException e) {
LOG.log(Level.SEVERE, "createServerSocket failed", new Object[] {type, port});
LOG.log(Level.SEVERE, "", e);
throw e;
}

try {
if (type.equals(SSL_MUTUALAUTH)) {
LOG.log(Level.FINE, "Setting Mutual auth");
((SSLServerSocket) ss).setNeedClientAuth(true);
}
} catch (Exception e) {
LOG.log(Level.SEVERE, "Setting Mutual auth failed.", e);
throw new IOException(e.getMessage());
}
if (LOG.isLoggable(Level.FINE)) {
LOG.log(Level.FINE, "Created server socket:" + ss);
throw new IOException(e.getMessage(), e);
}
LOG.log(Level.FINE, () -> "Created server socket: " + ss);
return ss;
}

Expand Down Expand Up @@ -424,9 +403,7 @@ private Socket createSSLSocket(String host, int port) throws IOException {
LOG.log(Level.FINE, "createSSLSocket failed.", new Object[] {host, port});
LOG.log(Level.FINE, "", e);
}
IOException e2 = new IOException("Error opening SSL socket to host=" + host + " port=" + port);
e2.initCause(e);
throw e2;
throw new IOException("Error opening SSL socket to host=" + host + " port=" + port, e);
}
return socket;
}
Expand Down Expand Up @@ -550,6 +527,20 @@ private boolean isValidProtocolCipher(CipherInfo cipherInfo,
}


private static <T> T repeat(Action<T> action, Duration timeout) throws IOException {
final Instant deadline = Instant.now().plus(timeout);
while (true) {
try {
return action.get();
} catch (Exception e) {
if (Instant.now().isAfter(deadline)) {
throw e;
}
}
}
}


class SSLInfo {
private final SSLContext ctx;
private String[] ssl3TlsCiphers = null;
Expand All @@ -573,4 +564,9 @@ String[] getSsl2Ciphers() {
return ssl2Ciphers;
}
}

@FunctionalInterface
private interface Action<T> {
T get() throws IOException;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation.
* Copyright (c) 2022, 2025 Contributors to the Eclipse Foundation.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -16,6 +16,8 @@

package org.glassfish.main.admin.test;

import com.sun.enterprise.util.OS;

import java.io.File;
import java.io.FileReader;
import java.io.LineNumberReader;
Expand Down Expand Up @@ -69,11 +71,16 @@ public class AsadminLoggingITest {

private static final Asadmin ASADMIN = GlassFishTestEnvironment.getAsadmin();

/** Fill up the server log. */
@BeforeAll
public static void fillUpServerLog() {
// Fill up the server log.
AsadminResult result = ASADMIN.exec("restart-domain", "--timeout", "60");
assertThat(result, asadminOK());
public static void fillUpServerLog() throws Exception {
if (OS.isWindowsForSure()) {
// For some reason windows can collide on debug port.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't experience blocked --debug ports on Linux in restart-domain command, but GHA on Windows does it. Seems like it doesn't close debug port when one process spawns another - the new then tries to open the same port, could reuse it, which is not a good idea, but ...
I think we had this problem on Solaris many years ago too.

assertThat(ASADMIN.exec("stop-domain"), asadminOK());
assertThat(ASADMIN.exec("start-domain"), asadminOK());
} else {
assertThat(ASADMIN.exec("restart-domain"), asadminOK());
}
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2024 Eclipse Foundation and/or its affiliates. All rights reserved.
* Copyright (c) 2024, 2025 Contributors to the Eclipse Foundation.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -13,6 +13,7 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
*/

package org.glassfish.main.admin.test;

import java.util.stream.Stream;
Expand All @@ -22,6 +23,7 @@
import org.glassfish.main.itest.tools.asadmin.AsadminResult;
import org.glassfish.main.itest.tools.asadmin.StartServ;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.extension.ExtensionContext;
Expand Down Expand Up @@ -54,29 +56,34 @@ static void setupDomain() {
}
}

@AfterEach
void stopDomain() {
ASADMIN.exec("stop-domain", STARTSERV_DOMAIN_NAME);
}

@AfterAll
static void deleteDomain() {
AsadminResult result = ASADMIN.exec("list-domains");
if (result.getOutput().contains(STARTSERV_DOMAIN_NAME)) {
ASADMIN.exec("delete-domain", STARTSERV_DOMAIN_NAME);
}
}

@ParameterizedTest
@ArgumentsSource(StartServArgumentsProvider.class)
public void startServerInForeground(StartServ startServ) {
try {
AsadminResult result = startServ.withTextToWaitFor("Total startup time including CLI").exec(STARTSERV_DOMAIN_NAME);
assertThat(result, asadminOK());
} finally {
ASADMIN.exec("stop-domain", STARTSERV_DOMAIN_NAME);
}
AsadminResult result = startServ.withTextToWaitFor("Total startup time including CLI").exec(STARTSERV_DOMAIN_NAME);
assertThat(result, asadminOK());
}

@ParameterizedTest
@ArgumentsSource(StartServArgumentsProvider.class)
@DisabledOnOs(value = WINDOWS, disabledReason = "startserv.bat is just trivial and doesn't give the error output")
public void reportCorrectErrorIfAlreadyRunning(StartServ startServ) {
try {
AsadminResult result = startServ.withTextToWaitFor("Total startup time including CLI").exec(STARTSERV_DOMAIN_NAME);
assertThat(result, asadminOK());
result = startServ.withNoTextToWaitFor().exec(STARTSERV_DOMAIN_NAME);
assertThat(result.getStdErr(), containsString("There is a process already using the admin port"));
} finally {
ASADMIN.exec("stop-domain", STARTSERV_DOMAIN_NAME);
}
AsadminResult result = startServ.withTextToWaitFor("Total startup time including CLI").exec(STARTSERV_DOMAIN_NAME);
assertThat(result, asadminOK());
result = startServ.withNoTextToWaitFor().exec(STARTSERV_DOMAIN_NAME);
assertThat(result.getStdErr(), containsString("There is a process already using the admin port"));
}

@ParameterizedTest
Expand All @@ -86,14 +93,6 @@ public void reportCorrectErrorIfInvalidCommand(StartServ startServ) {
assertThat(result.getStdErr(), containsString("There is no such domain directory"));
}

@AfterAll
static void deleteDomain() {
AsadminResult result = ASADMIN.exec("list-domains");
if (result.getOutput().contains(STARTSERV_DOMAIN_NAME)) {
ASADMIN.exec("delete-domain", STARTSERV_DOMAIN_NAME);
}
}

static class StartServArgumentsProvider implements ArgumentsProvider {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void jobSurvivesRestart() throws Exception {
"--cleanup-initial-delay=0s",
"--cleanup-poll-interval=0s"), asadminOK());
assertThat(ASADMIN.exec(COMMAND_PROGRESS_SIMPLE), asadminOK());
assertThat(ASADMIN.exec("restart-domain", "--timeout", "60"), asadminOK());
assertThat(ASADMIN.exec("restart-domain", "--timeout", "60", "domain1"), asadminOK());
assertThat(ASADMIN.exec("list-jobs").getStdOut(), stringContainsInOrder(COMMAND_PROGRESS_SIMPLE, "COMPLETED"));
JobTestExtension.doAndDisableJobCleanup();
}
Expand Down
Loading