On 05/07/2015 08:06 PM, Laurent Bourgès wrote:
Peter,
Thanks for long and detailled answer.
I know now better why OOME should not happen. However any application
may also use phantom references and the ReferenceHandler thread will
call Cleaner.run () which could catch OOME from the application code
implementing thunk.run (). Am I right ?
Any application may use PhantomReference(s) but not Cleaner(s). Cleaner
is a PhantoReference, but it is a JDK-internal API. I belive in JDK9 it
will not be visible at all (sun.misc.Cleaner). The Cleaner(s) are
reserved for JDK-internal use in particular because they utilize a
single ReferenceHandler thread that also serves a vital role of
dispatching of cleared Reference(s) to their ReferenceQueue(s)...
>> If this block also throws a new oome2 due to the first oome1 (no
memory left), it will work but I would have prefered a more explicit
solution and check oome1 first ...
I looked back at your patch and it is fine. Howevdr I wonder if it
would be possible to avoid any allocation in the catch(Throwable) block:
- preallocate the PriviledgeAction
- avoid new Error(x) to get its stack trace ? Do you know any trick
like ones in SharedSecrets that could dump the stack without any
allocation in case of urgency ?
What about the printing path? Who can guarantee that it doesn't use any
allocation? The diagnostic print-out that precedes System.exit() should
preferably be equipped with a stack-trace of the original exception.
Formatting a stack trace needs allocation.
But it's a good idea to try in that direction too. Perhaps 1st try to
print like now and if OOME #2 is thrown, resort to minimal printing that
doesn't allocate...
> You have a point and I asked myself the same question. The question
is how to treat OOME thrown from thunk.run(). Current behavior is to
exit() JVM for any exception (Throwable). I maintained that semantics.
I only added a handler for OOME thrown in the handler of the 1st
exception. I might have just exit()-ed the VM if OOME is thrown, but
leaving no trace and just exiting VM would not help anyone diagnose
what went wrong. So I opted for keeping the VM running for a while by
delaying the handling of 1st exception to "better times". If better
times never come, then the application is probably stuck anyway.
Seems very good after a 2nd look.
However, it could loop for a while if no more memory left ?
For example: oome1 => oome2 (catch) => throw x=> oome2 (catch) => ....
The retries are based on Cleaner processing code-path liveness. So it's
not really looping and doing nothing. Each time some Cleaner is found to
be processed, the pending exception is checked too. If no Cleaner(s) are
dequeued by ReferenceHandler thread after the one that 'saved' the
exception, the exception will not be handled and VM will not exit. I'm
aware of that, so your idea of trying to print something minimal without
allocation immediately if it can't be printed nicely, seems even more
attractive.
> An alternative would be to catch OOME from thunk.run() and ignore it
(printing it out would be ugly if VM is left to run), but that would
silently ignore OOMEs thrown from thunk.run() and noone would notice
that Cleaner(s) might not have clean-ed up the resources they should.
I am a bit lost but I like logging such exceptional case but if no
allocation can happen, how to ensure logging such case anyway ?
...
I must check printing code-path. Perhaps it doesn't need to allocate
anything.
> Anyway. If none of the Cleaner.thunk's run() methods can throw any
exception, then my handling of OOME is redundant and a code-path never
taken. But I would still leave it there in case some new Cleaner use
comes along which is not verified yet...
Agreed. It is always better to handle such exceptional case if you can
at least log them...
Best regards,
Laurent
Regards, Peter
