[WFCORE-5520] Avoid Nullpointer (Remoting)

[boris-unckel]

Fixes https://issues.redhat.com/browse/WFCORE-5520

[yersan]

connectorPropertiesOptionMap is also used similarly on L179, if finally this change is accepted, the if sentence on L179 should be modified accordingly.

[wildfly-ci]

Core - Full Integration Build 10917 outcome was FAILURE using a merge of 5a659c8
Summary: Tests failed: 1 (1 new), passed: 7538, ignored: 147 Build time: 03:19:09

Failed tests

org.jboss.as.test.clustering.cluster.singleton.SingletonDeploymentJBossAllTestCase.test: java.lang.AssertionError: expected:<200> but was:<404>
	at org.jboss.as.test.clustering.cluster.singleton.SingletonDeploymentTestCase.test(SingletonDeploymentTestCase.java:130)
------- Stdout: -------
Warning! The CLI is running in a non-modular environment and cannot load commands from management extensions.
 [0m11:45:22,505 INFO  [org.jboss.as.connector.subsystems.datasources] (MSC service thread 1-3) WFLYJCA0010: Unbound data source [java:jboss/datasources/ExampleDS]
 [0m [0m11:45:22,505 INFO  [org.jboss.as.mail.extension] (MSC service thread 1-3) WFLYMAIL0002: Unbound mail session [java:jboss/mail/Default]
 [0m [0m11:45:22,506 INFO  [org.jboss.modcluster] (ServerService Thread Pool -- 42) MODCLUSTER000002: Initiating mod_cluster shutdown
 [0m [33m11:45:22,508 WARN  [org.jboss.modcluster] (ServerService Thread Pool -- 42) MODCLUSTER000033: Failed to interrupt socket reception.: java.io.IOException: Invalid argument (sendto failed)
	at java.net.PlainDatagramSocketImpl.send(Native Method)
	at java.net.DatagramSocket.send(DatagramSocket.java:693)
	at org.jboss.modcluster.advertise.impl.AdvertiseListenerImpl.interruptDatagramReader(AdvertiseListenerImpl.java:197)
	at org.jboss.modcluster.advertise.impl.AdvertiseListenerImpl.stop(AdvertiseListenerImpl.java:212)
	at org.jboss.modcluster.advertise.impl.AdvertiseListenerImpl.destroy(AdvertiseListenerImpl.java:224)
	at org.jboss.modcluster.ModClusterService.shutdown(ModClusterService.java:199)
	at org.wildfly.extension.mod_cluster.ContainerEventHandlerServiceConfigurator.accept(ContainerEventHandlerServiceConfigurator.java:83)
	at org.wildfly.extension.mod_cluster.ContainerEventHandlerServiceConfigurator.accept(ContainerEventHandlerServiceConfigurator.java:50)
	at org.wildfly.clustering.service.FunctionalService.stop(FunctionalService.java:73)
	at org.wildfly.clustering.service.AsyncServiceConfigurator$AsyncService.lambda$stop$1(AsyncServiceConfigurator.java:142)
	at org.jboss.threads.ContextClassLoaderSavingRunnable.run(ContextClassLoaderSavingRunnable.java:35)
	at org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:1990)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask(EnhancedQueueExecutor.java:1486)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1377)
	at java.lang.Thread.run(Thread.java:748)
	at org.jboss.threads.JBossThread.run(JBossThread.java:513)

 [0m [0m11:45:22,509 INFO  [org.wildfly.extension.undertow] (MSC service thread 1-5) WFLYUT0008: Undertow HTTP listener default suspending
 [0m [0m11:45:22,509 INFO  [org.wildfly.extension.undertow] (MSC service thread 1-4) WFLYUT0008: Undertow HTTPS listener https suspending
 [0m [0m11:45:22,509 INFO  [org.wildfly.extension.undertow] (MSC service thread 1-2) WFLYUT0008: Undertow AJP listener ajp suspending
 [0m [0m11:45:22,511 INFO  [org.wildfly.extension.undertow] (MSC service thread 1-1) WFLYUT0019: Host default-host stopping
 [0m [0m11:45:22,511 INFO  [org.jboss.as.connector.deployers.jdbc] (MSC service thread 1-1) WFLYJCA0019: Stopped Driver service with driver-name = h2
 [0m [0m11:45:22,511 INFO  [org.wildfly.extension.undertow] (MSC service thread 1-2) WFLYUT0007: Undertow AJP listener ajp stopped, was bound to [::1]:8009
 [0m [0m11:45:22,512 INFO  [org.wildfly.extension.undertow] (MSC service thread 1-5) WFLYUT0007: Undertow HTTP listener default stopped, was bound to [::1]:8080
 [0m [0m11:45:22,512 INFO  [org.wildfly.extension.undertow] (MSC service thread 1-4) WFLYUT0007: Undertow HTTPS listener https stopped, was bound to [::1]:8443
 [0m [0m11:45:22,513 INFO  [org.wildfly.extension.undertow] (MSC service thread 1-4) WFLYUT0004: Undertow 2.2.10.Final stopping
 [0m [0m11:45:22,527 INFO  [org.jboss.as] (MSC service thread 1-7) WFLYSRV0050: WildFly Full 25.0.0.Beta1-SNAPSHOT (WildFly Core 17.0.0.Beta5-SNAPSHOT) stopped in 28ms
 [0m [0m11:45:22,527 INFO  [org.jboss.as] (MSC service thread 1-6) WFLYSRV0049: WildFly Full 25.0.0.Beta1-SNAPSHOT (WildFly Core 17.0.0.Beta5-SNAPSHOT) starting
 [0m [0m11:45:22,559 INFO  [org.jboss.as.controller.management-deprecated] (ServerService Thread Pool -- 7) WFLYCTL0033: Extension 'security' is deprecated and may not be supported in future versions
 [0m [0m11:45:22,580 INFO  [org.jboss.as.controller.management-deprecated] (Controller Boot Thread) WFLYCTL0028: Attribute 'security-realm' in the resource at address '/core-service=management/management-interface=http-interface' is deprecated, and may be removed in a future version. See the attribute description in the output of the read-resource-description operation to learn more about the deprecation.
 [0m [0m11:45:22,609 INFO  [org.jboss.as.controller.management-deprecated] (ServerService Thread Pool -- 40) WFLYCTL0028: Attribute 'security-realm' in the resource at address '/subsystem=undertow/server=default-server/https-listener=https' is deprecated, and may be removed in a future version. See the attribute description in the output of the read-resource-description operation to learn more about the deprecation.
 [0m [0m11:45:22,620 INFO  [org.jboss.as.server] (Controller Boot Thread) WFLYSRV0039: Creating http management service using socket-binding (management-http)
 [0m [0m11:45:22,640 INFO  [org.jboss.as.connector.subsystems.datasources] (ServerService Thread Pool -- 47) WFLYJCA0004: Deploying JDBC-compliant driver class org.h2.Driver (version 1.4)
 [0m [33m11:45:22,655 WARN  [org.wildfly.extension.elytron] (MSC service thread 1-3) WFLYELY00023: KeyStore file '/store/work/tc-work/37b47ae8b9c60325/full/testsuite/integration/clustering/target/wildfly-1/standalone/configuration/application.keystore' does not exist. Used blank.
 [0m [33m11:45:22,655 WARN  [org.wildfly.extension.elytron] (MSC service thread 1-3) WFLYELY01084: KeyStore /store/work/tc-work/37b47ae8b9c60325/full/testsuite/integration/clustering/target/wildfly-1/standalone/configuration/application.keystore not found, it will be auto generated on first use with a self-signed certificate for host localhost
 [0m [0m11:45:22,655 INFO  [org.wildfly.extension.health] (ServerService Thread Pool -- 55) WFLYHEALTH0001: Activating Base Health Subsystem
 [0m [0m11:45:22,657 INFO  [org.jboss.as.clustering.infinispan] (ServerService Thread Pool -- 56) WFLYCLINF0001: Activating Infinispan subsystem.
 [0m [0m11:45:22,657 INFO  [org.wildfly.extension.io] (ServerService Thread Pool -- 57) WFLYIO001: Worker 'default' has auto-configured to 8 IO threads with 64 max task threads based on your 4 available processors
 [0m [0m11:45:22,659 INFO  [org.jboss.as.jaxrs] (ServerService Thread Pool -- 58) WFLYRS0016: RESTEasy version 3.15.1.Final
 [0m [0m11:45:22,660 INFO  [org.jboss.as.clustering.jgroups] (ServerService Thread Pool -- 61) WFLYCLJG0001: Activating JGroups subsystem. JGroups version 4.2.11
 [0m [0m11:45:22,661 INFO  [org.wildfly.extension.metrics] (ServerService Thread Pool -- 66) WFLYMETRICS0001: Activating Base Metrics Subsystem
 [0m [0m11:45:22,662 INFO  [org.jboss.as.connector] (MSC service thread 1-1) WFLYJCA0009: Starting Jakarta Connectors Subsystem (WildFly/IronJacamar @VERSION@)
 [0m [0m11:45:22,662 INFO  [org.wildfly.extension.microprofile.config.smallrye] (ServerService Thread Pool -- 67) WFLYCONF0001: Activating MicroProfile Config Subsystem

[boris-unckel]

connectorPropertiesOptionMap is also used similarly on L179, if finally this change is accepted, the if sentence on L179 should be modified accordingly.

@yersan thanks for your feedback. I have changed the PR accordingly. Please check again.

[yersan]

Hello @boris-unckel , looking more carefully at the source code, I don't believe that we are going to have a connectorPropertiesOptionMap as null in this piece of code. All the callers pass a non-null value.

In any case, preventing for null is fine and there is an if around L152 that does that. Instead of adding a repetitive null check for the same instance as you did on your changes, what about replacing the uses of connectorPropertiesOptionMap with resultingMap after the initial null check on L152 and avoid the repetitive null checks? What do you think?

If connectorPropertiesOptionMap is not null, then resultingMap will contain their values. If connectorPropertiesOptionMap is null, then resultingMap will be empty, so there is no need for further null checks. It seems that would make the code cleaner and will avoid repeating check if connectorPropertiesOptionMap when it is used along with this method.

[boris-unckel]

Hello @yersan please have a look at this version. It relies on resultingMap now.

[yersan]

Sorry @boris-unckel , this PR can be closed, we have been working on a piece of code that was going to be removed, see #4716

[boris-unckel]

@yersan Anyway no problem anymore. Thanks for your ongoing support. 👍