JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

All,

Please see comments inline.

Thanks,

Brian

On Oct 1, 2013, at 8:44 PM, Alan Bateman wrote:

> On 01/10/2013 12:46, Brian Burkhalter wrote:
>> :
>> 
>> I updated the webrev
>> 
>> http://cr.openjdk.java.net/~bpb/8010371/
>> 
>> with changes in the test of the return value of getaddrinfo for Unix Inet 4 and 6 and Windows Inet 6. The usual testing is in progress.
>> 
>> Brian
> This looks better, although I think I would reverse re-write the expressions to "if (error = ...)".

I agree that is an ugly style but it guarantees that mistakes such as

	if (error = EAI_AGAIN) {}

are caught at compilation time.

> One thing to consider is whether this condition is really worth introducing HostLookupException, particularly when it doesn't include additional information (except to distinguish it from its supertype). If a new exception is really needed then maybe it could add the error message obtained from gai_strerror, alternatively maybe you could considered setting the cause of the UHE to something like an IOException with the translation of the error.

On Oct 2, 2013, at 7:40 AM, Chris Hegarty wrote:

> On 02/10/2013 04:44, Alan Bateman wrote:
>> 
>> One thing to consider is whether this condition is really worth
>> introducing HostLookupException, particularly when it doesn't include
> 
> I am also not convinced of the merits of adding a new public checked Exception type for this situation. Do we expect all callers of API's that can throw UKE to now have to catch this Exception and provide their own try logic? Otherwise, what do we expect them to do with it.
> 
> I cannot see the original webrev, so cannot comment on the retry logic that was there. Do you still have it around? Can upload as a .05 version? The reason I ask is that, IMHO, I would prefer this type of approach over the new Exception type, but that could be just me.

No, I do not have it.

On Oct 2, 2013, at 7:58 AM, Michael McMahon wrote:

> On 02/10/13 15:40, Chris Hegarty wrote:
>> 
>> I am also not convinced of the merits of adding a new public checked Exception type for this situation. Do we expect all callers of API's that can throw UKE to now have to catch this Exception and provide their own try logic? Otherwise, what do we expect them to do with it.
>> 
> 
> It's proposed as a subclass of UnknownHostException. So, nobody would have to catch it.
> 
> I suppose another approach, which is a variant of the first one suggested, would be
> retry logic built in, which is switched off by default and configurable somehow.

I personally do not like the idea of the configurable approach.

On Oct 2, 2013, at 8:12 AM, Chris Hegarty wrote:

> On 02/10/2013 15:58, Michael McMahon wrote:
>> 
>> It's proposed as a subclass of UnknownHostException. So, nobody would
>> have to catch it.
> 
> Right, understood. But unless someone adds explicit handling code when there is no change in existing behavior.

Well this is good in the sense that existing code would not be affected negatively but it would require that code be changed for the application explicitly to do a retry.

>> I suppose another approach, which is a variant of the first one
>> suggested, would be
>> retry logic built in, which is switched off by default and configurable
>> somehow.
> 
> I did not see the retry webrev so cannot comment on the specifics, but I don't see any issue with a single untimed retry, if that fails throw an UHE with a specific cause.

This is close to what I original thought to do modulo the exception class.

So, how about this approach:

1) If the error is EAI_AGAIN / EIA_SYSTEM+EAGAIN / WSATRY_AGAIN then do one immediate native retry.
2) If the retry fails with the same error, then throw a UHE with a specific message or cause.

Questions:

A) Would it be better to do the retry in the Java layer, perhaps with a very short wait?
B) Should the message or cause in #2 be explicitly document in the javadoc?