Copilot commented on code in PR #2521:
URL: https://github.com/apache/tomee/pull/2521#discussion_r2852887195
##########
server/openejb-client/src/main/java/org/apache/openejb/client/MulticastPulseClient.java:
##########
@@ -677,73 +671,69 @@ public static void main(final String[] args) throws
Exception {
final AtomicBoolean running = new AtomicBoolean(true);
- final Thread t = new Thread(new Runnable() {
- @SuppressWarnings("UseOfSystemOutOrSystemErr")
- @Override
- public void run() {
- while (running.get()) {
+ final Thread t = new Thread(() -> {
+ while (running.get()) {
- Set<URI> uriSet = null;
- try {
- uriSet = MulticastPulseClient.discoverURIs(discover,
new HashSet<String>(Arrays.asList("ejbd", "ejbds", "http", "https")), mchost,
mcport, timeout);
- } catch (Exception e) {
- System.err.println(e.getMessage());
- }
+ Set<URI> uriSet = null;
+ try {
+ uriSet = MulticastPulseClient.discoverURIs(discover, new
HashSet<String>(Arrays.asList("ejbd", "ejbds", "http", "https")), mchost,
mcport, timeout);
+ } catch (Exception e) {
+ System.err.println(e.getMessage());
+ }
- final int size = uriSet.size();
- if (uriSet != null && size > 0) {
+ final int size = uriSet.size();
+ if (uriSet != null && size > 0) {
Review Comment:
`uriSet` is dereferenced (via `uriSet.size()`) before the null check. If
`discoverURIs(...)` throws and leaves `uriSet` as null, this thread will NPE
and stop discovery. Move the `size` computation after the null check (or
default `uriSet` to `Collections.emptySet()`).
##########
server/openejb-client/src/main/java/org/apache/openejb/client/proxy/ProxyManager.java:
##########
@@ -120,12 +121,7 @@ public static Object newProxyInstance(final Class
proxyClass) throws IllegalAcce
}
public static ClassLoader getContextClassLoader() {
- return (ClassLoader) java.security.AccessController.doPrivileged(new
java.security.PrivilegedAction() {
-
@Override
-
public Object run() {
-
return Thread.currentThread().getContextClassLoader();
- }
- }
+ return (ClassLoader)
java.security.AccessController.doPrivileged((PrivilegedAction) () ->
Thread.currentThread().getContextClassLoader()
);
}
Review Comment:
`doPrivileged` is invoked with a raw `PrivilegedAction` and then cast back
to `ClassLoader`, which introduces unchecked warnings and hides the expected
return type. Prefer
`AccessController.doPrivileged((PrivilegedAction<ClassLoader>)
Thread.currentThread()::getContextClassLoader)` (or equivalent) so the return
type is explicit and the outer cast can be removed.
##########
arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/RemoteInitialContextObserver.java:
##########
@@ -89,25 +89,13 @@ public MultipleContextHandler(final Properties props, final
Context initialConte
@Override
public Object invoke(final Object proxy, final Method method, final
Object[] args) throws Throwable {
Exception err = null;
+ // then contextual context, this can start an embedded container
in some cases
+ // then existing context
+ // then try to create a remote context
for (final Callable<Context> callable : Arrays.asList( // order is
important to avoid to start an embedded container for some cases
Review Comment:
The new comment describes an order (contextual -> existing -> remote) that
doesn't match the actual `Arrays.asList(...)` order (remote -> existing ->
contextual). Since the order is stated as important, please either adjust the
comment to reflect the current order or reorder the callables to match the
comment to avoid future regressions.
```suggestion
// first, try to create a remote context using the provided
properties
// then, use the existing context if available
// finally, fall back to a default/contextual InitialContext
(may start an embedded container)
for (final Callable<Context> callable : Arrays.asList( // order
is important to avoid starting an embedded container in some cases
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
