Fix #244 NullPointerException in new InitialContext() if IIOP endpoints are unreachable

[tnagao7]

Fixes #244

This pull request addresses NullPointerException that occurs when new InitialContext() is called with unreachable IIOP endpoints.
It adds null checks before creating ArrayList instances from the results of internalClusterInstanceInfo(), preventing the exception when the returned collection is null.

[pzygielo]

For reference

orb/appserver/orb/orb-iiop/src/main/java/org/glassfish/enterprise/iiop/impl/GroupInfoServiceObserverImpl.java

Lines 67 to 69 in c34aca8

	if (instanceInfoList != null && !instanceInfoList.isEmpty()) {
	rr.setClusterInstanceInfo(instanceInfoList);
	}

would accept null as well.

[tnagao7]

would accept null as well.

So are you suggesting that getClusterInstanceInfo should return null instead of an empty array?
If so the changes in this PR would be like:

-    return new ArrayList( internalClusterInstanceInfo() ) ;
+    List<ClusterInstanceInfo> internalInfo = internalClusterInstanceInfo();
+    return internalInfo == null ? null : new ArrayList<>(internalInfo);

[pzygielo]

So are you suggesting that getClusterInstanceInfo should return null instead of an empty array?

No, I'm not. I think your decision to return empty collection is the right one. What I meant is, that on GF side it was somehow anticipated that null might be returned (maybe it was at one point, I didn't investigate history here), and on ORB side this could have been implemented then. Instead, it was overlooked and allowed NPEx to happen.

[pzygielo]

So this looks fine for me, but I'd like to check implementation(s) of public abstract List<ClusterInstanceInfo> internalClusterInstanceInfo( List<String> endpoints ) ; - why null is returned from it.

And maybe public List<ClusterInstanceInfo> internalClusterInstanceInfo() could also return empty list for consistency with this PR changes.

[pzygielo]

why null is returned from it.

orb/orbmain/src/main/java/com/sun/corba/ee/impl/folb/ClientGroupManager.java

Lines 486 to 489 in 018a2b9

	} catch (Exception e) {
	reportException(e);
	return null;
	}

due to org.omg.CORBA.COMM_FAILURE: FINE: 00410001: Connection failure: socketType: IIOP_CLEAR_TEXT; hostname: dummyhost; port: 3700 vmcid: OMG minor code: 1 completed: No.

orb/orbmain/src/main/java/com/sun/corba/ee/impl/folb/ClientGroupManager.java

Line 110 in 018a2b9

private void reportException( Exception exc ) { }

[tnagao7]

And maybe public List<ClusterInstanceInfo> internalClusterInstanceInfo() could also return empty list for consistency with this PR changes.

Here are the lists for return values of relevant methods.

GroupInfoService#getClusterInstanceInfo methods:

No.	Repo	Class	Method	Returns null in certain cases
1-1	orb	GroupInfoService	getClusterInstanceInfo(String[] adapterName)	Depends on the subclass implementation.
1-2	orb	GroupInfoService	getClusterInstanceInfo(String[] adapterName, List<String> endpoints)	Depends on the subclass implementation.
1-3	orb	GroupInfoServiceBase	getClusterInstanceInfo(String[] adapterName)	No
1-4	orb	GroupInfoServiceBase	getClusterInstanceInfo(String[] adapterName, List<String> endpoints )	No

InitialGroupInfoService.InitialGIS#getClusterInstanceInfo methods:

No.	Repo	Class	Method	Returns null in certain cases
2-1	orb	InitialGroupInfoService.InitialGIS	getClusterInstanceInfo()	Depends on the subclass implementation.
2-2	orb	InitialGroupInfoService.InitialGISImpl	getClusterInstanceInfo()	Yes. Returns null if an exception occurs.

GroupInfoServiceBase#internalClusterInstanceInfo methods:

No.	Repo	Class	Method	Returns null in certain cases
3-1	orb	GroupInfoServiceBase	internalClusterInstanceInfo()	Depends on the subclass implementation.
3-2	orb	GroupInfoServiceBase	internalClusterInstanceInfo( List<String> endpoints )	Depends on the subclass implementation.
3-3	orb	ClientGroupmanager.GIS	internalClusterInstanceInfo( List<String> endpoints )	Yes. Returns null if initGIS.getClusterInstanceInfo() returns null or an exception occurs in getInitialClusterInstanceInfo.
3-4	orb	GroupInfoServiceImpl.GIS	internalClusterInstanceInfo( List<String> endpoints )	No. Always throws RuntimeException.
3-5	glassfish	IiopFolbGmsClient. GroupInfoServiceGmsImpl	internalClusterInstanceInfo( List<String> endpoints )	No. Returns an empty list if no members are available.

The current situation can be explained as follows:

• GroupInfoService#getClusterInstanceInfo (No. 1-x) never returns null. There are caller classes (e.g., ServerGroupManager) that expect GroupInfoService#getClusterInstanceInfo not to return null.
• Auxiliary methods (No. 2-x, 3-x) used in GroupInfoService#getClusterInstanceInfo (No. 1-x) return null if exceptions are thrown.

I found that there is another method (No. 2-2) returning null in addition to public List<ClusterInstanceInfo> internalClusterInstanceInfo() (No. 3-3).
The current implementation of them (returning null in an error situation) seems natural itself, making it difficult to determine whether to change them.
If you think that both (No. 2-2 and 3-3) should return an empty list, I can implement this.

[pzygielo]

Nice summary, thank you.

The current implementation of them (returning null in an error situation) seems natural itself, making it difficult to determine whether to change them.

I do agree.

And with that - for the changes in PR I think, which means I'm not quite sure (as I like empty collection being returned) - perhaps nulls should go out of modified methods? (With swallowed exception there is no other way to signal that to client.)

@dmatej could you check this, please?