RFR: 8152515: (logging) LogManager.resetLogger should ignore LinkageError

[Daniel Fuchs]

On 28/10/16 20:32, David Holmes wrote:
> Hi Daniel,
>
> I've read the bug report on this and this issue "smells" to me.
> LinkageError should not be a special case IMHO. The existing code was
> trying to go the "ignore everything but 'serious errors' " route - but
> without really considering what constitutes a "serious error". I think
> the fact a LinkageError can turn up here reflects a bug in the code that
> throws it - or possibly in something in between.
>
> There should be a very clear exception handling policy for this code,
> and not special cases IMHO. Having that policy depend on whether
> shutdown is in progress is okay to me as we know that things can behave
> unexpectedly in that case. So I would advocate for a more general
>
> +             } catch (Throwable e) {
> +                 // Ignore all exceptions while shutting down
> +                 if (globalHandlersState != STATE_SHUTDOWN) {
> +                     throw e;
> +                 }
>

Hi David,

This is something I had been considering too.

In the end it made me feel uneasy to trap all errors - like OOME
or IllegalAccessError (swallowing IAE is not great when
one migrates from JDK 8 to JDK 9 where IAE helps diagnose
encapsulation issues) - so I opted for the minimal fix.

I agree that just trapping LinkageError is a bit strange,
but we do have a use case for that - which AFAIU is not
uncommon.

Anyway, if you ask me then the root of the problem is
that the various shutdown hook can run in parallel and in
unspecified order - so making the reset() more robust by
trapping all errors might not be such a bad idea after all...
Here is an updated webrev:

http://cr.openjdk.java.net/~dfuchs/webrev_8152515/webrev.02/


              } catch (Exception ex) {
                  // Problems closing a handler?  Keep going...
+            } catch (Error e) {
+                // ignore Errors while shutting down
+                if (globalHandlersState != STATE_SHUTDOWN) {
+                    throw e;
+                }
              }

best regards,

-- daniel


> Cheers,
> David
>
> On 28/10/2016 9:51 PM, Daniel Fuchs wrote:
>> Hi,
>>
>> Please find below a trivial  patch for:
>>
>> 8152515: (logging) LogManager.resetLogger should ignore LinkageError
>> https://bugs.openjdk.java.net/browse/JDK-8152515
>>
>>
>> Patch:
>> http://cr.openjdk.java.net/~dfuchs/webrev_8152515/webrev.00/
>>
>> The issue might occur at shutdown, when a handler that makes uses
>> of some APIs provided by an OSGI bundle which was already closed
>> by the shutdown process is in turn closed by the LogManager.Cleaner
>> thread. In that case some subclasses of LinkageError may be thrown,
>> interrupting the reset process and preventing other handlers from
>> being closed properly.
>>
>> The patch proposes to trivially ignore LinkageError at shutdown while
>> the LogManager.Cleaner thread is running.
>>
>> best regards,
>>
>> -- daniel