On 05/06/2013 10:02 AM, Thomas Schatzl wrote:
Hi,
On Sat, 2013-05-04 at 21:23 +0200, Peter Levart wrote:
On 05/04/2013 07:38 PM, Vitaly Davidovich wrote:
Oops, that was my mistake - I thought the lock here was a j.u.c.Lock
which of course doesn't even make sense given we're talking about
ObjectMonitor. So disregard that bit.
Ignoring OOM and continuing just seems very fragile as it means you
somehow know that all state is still consistent. Most java code
isn't async exception safe, so it's hard to reason about state after
OOM. Maybe Reference Handler is OK in that regard though.
I think it is safe to ignore OOME here, since this is the only place
As my test program shows, this may not be the only place where heap
allocation can happen within that code.
Alan also mentioned something about instrumentation that can add memory
allocations basically anywhere.
As the reference handler code is plain java code, it will be affected as
other java code.
where heap allocation happens and it is known what provokes it.
Otherwise ReferenceHandler just shuffles existing pointers in existing
objects...
In the current compilers, yes. But what about other compilers/VMs that
may add more elaborate profiling information here and there? E.g. the
graal compiler? Not sure if it does, probably it does not, but it seems
shaky to rely on compiler code generation here.
So is it possible to handle all known issues (especially if the fix is
similar/known/understandable), not just the original one, here?
I mean, if we fix this issue as you suggested (I am not against that, it
looks reasonable), I would not know what to do with the test program
except file another bug against the very same component with the same
problem, with the same fix suggestion.
I.e. the code still seems to buggy even after applying that fix, and
with instrumentation I think I could create another reproducer. As
reference processing is something critical to the VM, I think it is
prudent to fix all known problems here and now if possible.
Maybe there is some argument against simply wrapping the entire loop
with a try-catch? Please elaborate if so (maybe I overlooked that?), or
maybe you could suggest another solution?
Additionally, just fixing only the exact issue here seems somewhat
wasteful in terms of time (imho) - because next time another developer
will likely spend lots of time trying to reproduce this again. (As
mentioned, the former assignee also seemed to be unable to reproduce
this for a long time).
Thanks,
Thomas
Hi Thomas,
I agree with you. If instrumentation/different JVM can throw OOME
practicaly anywhere, then we have a problem with every such "critical"
piece of code that is written in Java. What if instrumentation inserts
allocation at the start of each loop? You might think this could be
fixed by:
for (;;) {
// instrumentation added allocation #1 here
try {
for (;;) {
// instrumentation added allocation #2 here
...
}
}
catch (OutOfMemoryError ignore) {}
}
Say you successfully enter inner-loop and run it for a while. Then a
heap shortage situation happens and "allocation #2" throws OOME, which
is caught and ignored. In next iteration of outer-loop "allocation #1"
will probably throw OOME too, since heap is probably still full...
To be 100% sure such "critical" loop never ends, I only see two options:
1. move such loop to different environment (native code?) where
allocations are more predictable or
2. add facility to Java/JVM that can mark portions of Java code where
allocation happens more predictably
The 2nd option could be specified as a kind of annotation on a method,
for example:
private static class ReferenceHandler extends Thread {
@DontInstrument
public void run() {
...
}
For the 1st option, we could create a native counterpart of the
following static method:
public class CriticalLoop {
public static void doForever(Runnable runnable) {
for (;;) {
try {
runnable.run();
}
catch (OutOfMemoryError) {
// wait a little uninterruptibly
}
}
}
}
...and use it in ReferenceHandler to run the critical loop.
Regards, Peter
