You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Christopher Schultz <ch...@christopherschultz.net> on 2014/07/14 17:01:02 UTC

ContainerBackgroundProcessor and compounding OOMEs

All,

It seems that ContainerBackgroundProcessor can die and end up silently
destabilizing things.

I got this in a development environment today:

Exception in thread
"ContainerBackgroundProcessor[StandardEngine[Catalina]]"
java.lang.OutOfMemoryError: GC overhead limit exceeded
        at
java.lang.StringCoding$StringEncoder.encode(StringCoding.java:300)
        at java.lang.StringCoding.encode(StringCoding.java:344)
        at java.lang.String.getBytes(String.java:916)
        at java.io.UnixFileSystem.getLastModifiedTime(Native Method)
        at java.io.File.lastModified(File.java:937)
        at
org.apache.naming.resources.FileDirContext$FileResourceAttributes.get
LastModified(FileDirContext.java:1008)
        at
org.apache.naming.resources.FileDirContext$FileResourceAttributes.get
Creation(FileDirContext.java:980)
        at
org.apache.naming.resources.FileDirContext$FileResourceAttributes.<in
it>(FileDirContext.java:925)
        at
org.apache.naming.resources.FileDirContext.doGetAttributes(FileDirCon
text.java:403)
        at
org.apache.naming.resources.BaseDirContext.getAttributes(BaseDirConte
xt.java:1137)
        at
org.apache.naming.resources.BaseDirContext.getAttributes(BaseDirConte
xt.java:1090)
        at
org.apache.naming.resources.ProxyDirContext.getAttributes(ProxyDirCon
text.java:882)

Exception: java.lang.OutOfMemoryError thrown from the
UncaughtExceptionHandler in thread
"ContainerBackgroundProcessor[StandardEngine[Catalina]]"

Okay, this kind of thing sometimes happens in our development instance.
But what I noticed is that the ContainerBackgroundProcessor thread has
not died.

That means that even if the OOME was transient and the JVM could recover
from the failure, the background processor thread is dead and things
like sessions will pile up until memory is truly exhausted.

The problem is in the code for the runnable method. Somewhat simplified,
it's just a loop to do stuff:

while(!done) {
  try {
    sleep();
    processChildren();
  } catch (Throwable t) {
    ExceptionUtils.handleException(t);
    log("error", t);
  }
}

Although the stack trace doesn't show it, the above error clearly
occurred in processChildren().

ExceptionUtils.handleException checks for two things that I think we
might want to change:

1. If the exception is StackOverflowError, it silently ignores the error
and continues. I think we should at least log something, probably at
WARN level.

2. If the exception is VirtualMachineError, it gets re-thrown with no
log. This skips the "log" call in the above code and so the only log
will come from the VM's "unhandled exception" logger which may not go
where you expect it to go. The exception propagates, and the thread's
run() method finishes (escapes due to uncaught exception). After that,
regardless of the recoverability of the situation (OOME), the background
processor will not run and therefore no auto-reload applications will
auto-reload, no sessions will ever die, etc.

If we think that StackOverflowError is recoverable, why not OutOfMemory?
What about other VirtualMachineErrors?

-chris


Re: ContainerBackgroundProcessor and compounding OOMEs

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Konstantin,

On 7/17/14, 7:58 AM, Konstantin Kolinko wrote:
> 2014-07-15 23:43 GMT+04:00 Christopher Schultz <ch...@christopherschultz.net>:
>> Mark,
>>
>> (...)
>>
>> We have had situations /in production/ where a "single thread" has
>> triggered an OOME .. in a particular case, that thread was trying to
>> pull an unbounded set of results back from a database query and cache
>> them in a local data structure. The OOME killed the operation, the GC
>> kicked-in, trashed that huge structure, and the system stabilized.
>>
>> We didn't want to bounce the server in the middle of the day and
>> interrupt people if we didn't have to. We watched things closely and
>> were convinced that it was in fact stable. We waited for the weekend and
>> bounced the JVM -- just to be "safe".
>>
>> Had the background thread died in that situation, we would have had to
>> bounce the JVM within about 30 minutes given our user load since the
>> sessions would never die. We also have to meet certain "security
>> requirements" about expiration of idle connections, and without the
>> background thread, we cannot enforce those rules. (Note that, when the
>> background thread is dead, you can't even use JMX to ask that expired
>> sessions be expired: the 'backgroundProcess' operation seems to try to
>> nudge the background processor which .. isn't around anymore).
>>
>> This discussion has become even more academic since Konstantin has begun
>> a bit of work already to help alleviate this. Thanks, K.
>>
> 
> It wouldn't be committed unless there is some consensus. Real-life
> examples are important to evaluate the feature.
> 
> Well, at least the logging part of the patch can be useful.
> 
> Note that ContainerBase#threadStart() protects itself from repeated
> calls with "if (thread!=null)" check.
> 
> If you add  try{}finally{ thread=null; } to
> ContainerBackgroundProcessor.run() then it will be possible to
> explicitly call threadStart() to restart the thread.

It's probably worth doing, even if we don't expose a "restart" method
just yet.

> It may be possible to expose threadStart() to JMX. (Though maybe
> better to create a separate public method. E.g. one that walks all
> children and tries to start their threads). It needs some testing
> whether that is feasible.
> 
> An issue with automated restart of background thread is that the
> restarted thread is likely to perform the same work. As thus, it is
> likely to meet the same stackoverflowexception a few seconds after it
> has been started.

Agreed, but this is something that wouldn't necessarily be automatic: an
admin can make the determination of whether to restart the thread or not
and observe the results.

> Note also that you can configure a separate background thread in each
> container. You may configure to have separate thread in Host (doing
> autodeployment) and separate threads in Contexts (doing session
> expiration work).

I would be happy if the Manager's "backgroundProcess" JMX operation
would simply run the background process one single time, synchronously
or not. Right now, it ties into the background-processing lifecycle such
that it won't necessarily expire any sessions on a given execution.
Perhaps what I'm really looking for is direct access to the
processExpires() method.

-chris


Re: ContainerBackgroundProcessor and compounding OOMEs

Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-07-15 23:43 GMT+04:00 Christopher Schultz <ch...@christopherschultz.net>:
> Mark,
>
> (...)
>
> We have had situations /in production/ where a "single thread" has
> triggered an OOME .. in a particular case, that thread was trying to
> pull an unbounded set of results back from a database query and cache
> them in a local data structure. The OOME killed the operation, the GC
> kicked-in, trashed that huge structure, and the system stabilized.
>
> We didn't want to bounce the server in the middle of the day and
> interrupt people if we didn't have to. We watched things closely and
> were convinced that it was in fact stable. We waited for the weekend and
> bounced the JVM -- just to be "safe".
>
> Had the background thread died in that situation, we would have had to
> bounce the JVM within about 30 minutes given our user load since the
> sessions would never die. We also have to meet certain "security
> requirements" about expiration of idle connections, and without the
> background thread, we cannot enforce those rules. (Note that, when the
> background thread is dead, you can't even use JMX to ask that expired
> sessions be expired: the 'backgroundProcess' operation seems to try to
> nudge the background processor which .. isn't around anymore).
>
> This discussion has become even more academic since Konstantin has begun
> a bit of work already to help alleviate this. Thanks, K.
>

It wouldn't be committed unless there is some consensus. Real-life
examples are important to evaluate the feature.

Well, at least the logging part of the patch can be useful.

Note that ContainerBase#threadStart() protects itself from repeated
calls with "if (thread!=null)" check.

If you add  try{}finally{ thread=null; } to
ContainerBackgroundProcessor.run() then it will be possible to
explicitly call threadStart() to restart the thread.

It may be possible to expose threadStart() to JMX. (Though maybe
better to create a separate public method. E.g. one that walks all
children and tries to start their threads). It needs some testing
whether that is feasible.


An issue with automated restart of background thread is that the
restarted thread is likely to perform the same work. As thus, it is
likely to meet the same stackoverflowexception a few seconds after it
has been started.

Note also that you can configure a separate background thread in each
container. You may configure to have separate thread in Host (doing
autodeployment) and separate threads in Contexts (doing session
expiration work).

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: ContainerBackgroundProcessor and compounding OOMEs

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,

On 7/15/14, 12:44 PM, Mark Thomas wrote:
> On 14 July 2014 21:58:22 CEST, Christopher Schultz <ch...@christopherschultz.net> wrote:
>> On 7/14/14, 12:18 PM, Mark Thomas wrote:
> 
>>>> 2. If the exception is VirtualMachineError, it gets re-thrown with
>>>> no log. This skips the "log" call in the above code and so the only
>>>> log will come from the VM's "unhandled exception" logger which may
>>>> not go where you expect it to go.
>>>
>>> It goes to stderr unless they system admin has redirected it and if
>>> they have, they should know where to look for it. Further, they
>> should
>>> also be monitoring it.
>>
>> True, but the background thread does die.
> 
> So? The VM is now in an unknown and potentially unstable state. Restarting the VM is the only sensible way forward so the fact that the background thread has died and any negative consequences of that are irrelevant.
> 
>>>> If we think that StackOverflowError is recoverable, why not
>>>> OutOfMemory?
>>>
>>> There is no assumption that StackOverflowError is recoverable.
>>
>>    public static void handleThrowable(Throwable t) {
>>        if (t instanceof ThreadDeath) {
>>            throw (ThreadDeath) t;
>>        }
>>        if (t instanceof StackOverflowError) {
>>            // Swallow silently - it should be recoverable
>>            return;
>>        }
>>        if (t instanceof VirtualMachineError) {
>>            throw (VirtualMachineError) t;
>>        }
>>        // All other instances of Throwable will be silently swallowed
>>    }
>>
>> As of April 17, 2014, markt disagrees with you (r1588269):
> 
> I disagree with him all the time. In this case though I think it is just context and that probably just needs a better comment.
> 
> Stack overflow in application code should be recoverable as far as Tomcat is concerned. The implication for the application that triggered it may be more severe. There is no assumption that it is recoverable for the application (hence the ERROR level log) but there is a possibility that it might be recoverable therefore - like just about every other application level error - Tomcat carries on.
> 
> 
>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/util/ExceptionUtils.java?r1=1588269&r2=1588268&pathrev=1588269
>>
>>> That is why it is logged at ERROR level.
>>
>> You are right, StackOverflow gets logged, but things like OOME are not
>> -- they are left to the unhandled exception handler. But they do
>> take-down the background thread when they do.
> 
> Other VM errors are assumed to be fatal. Hence I don't care if the background processing thread dies in this case.
> 
> 
>>> It is guaranteed that only that
>>> thread was affected so you know (from the stack trace) exactly what
>>> failed where and you can also determine how bad things are and opt to
>>> restart Tomcat if necessary.
>>
>> For OOME, if you are using sessions you will need to schedule a restart
>> pretty much immediately, since they will never die.
>>
>>> With respect to OOME how, exactly, do you propose to differentiate
>>> between a "recoverable" OOME and a non-recoverable one?
>>
>> If the admin is monitoring (which we are), then they can make the
>> determination. The current situation is that Tomcat needs to be bounced
>> regardless of the severity of the OOME, because the background thread
>> stops.
> 
> I disagree. You have no way of knowing the state of the VM after a OOME. You can hope the VM is OK but the only safe option is a restart.
> 
>>>> What about other VirtualMachineErrors?
>>>
>>> The current position is that they are all non-recoverable. If it can
>>> be demonstrated that more of them should be treated like
>> StackOverflow
>>> then we can do so.
>>
>> Okay. I think OOME should be treated similarly to StackOverflowError.
> 
> I strongly disagree. A stack overflow is guaranteed to only affect
> the thread that reports the error. It is my understanding that no
> such guarantee exists for an OOME. I'm prepared to be convinced
> otherwise (eg by something in one of the Java specs) but until that
> happens I am -1 on treating OOME like a stack overflow.

We have had situations /in production/ where a "single thread" has
triggered an OOME .. in a particular case, that thread was trying to
pull an unbounded set of results back from a database query and cache
them in a local data structure. The OOME killed the operation, the GC
kicked-in, trashed that huge structure, and the system stabilized.

We didn't want to bounce the server in the middle of the day and
interrupt people if we didn't have to. We watched things closely and
were convinced that it was in fact stable. We waited for the weekend and
bounced the JVM -- just to be "safe".

Had the background thread died in that situation, we would have had to
bounce the JVM within about 30 minutes given our user load since the
sessions would never die. We also have to meet certain "security
requirements" about expiration of idle connections, and without the
background thread, we cannot enforce those rules. (Note that, when the
background thread is dead, you can't even use JMX to ask that expired
sessions be expired: the 'backgroundProcess' operation seems to try to
nudge the background processor which .. isn't around anymore).

This discussion has become even more academic since Konstantin has begun
a bit of work already to help alleviate this. Thanks, K.

> I'll add that if it can be demonstrated that any of the other virtual
> machine errors only affect the current thread and do not indicate
> something is fundamentally broken in the VM then I'd be happy to
> treat them the same way as a stack overflow. (Given what virtual
> machine error is used for I think this is unlikely).

I checked, and I think you are right: VME is basically StackOverFlow,
OOME, and "holy shit we have no idea what in god's name is happening
here". I definitely agree that the latter cases should be treated as
completely catastrophic and the background processor can die for all I
care. (Note that I've never see any of those other class of errors in
the real world: perhaps I'll be upset when they take down by Tomcat
background thread in the future ;)

> I'll also add that if the current blanket 'allowing' of stack
> overflow can be shown to be harmful in some cases then I'd be in
> favour of changing that handling to not re throw the stack overflow
> only in those cases where it is known to be safe to do so.

-chris



Re: ContainerBackgroundProcessor and compounding OOMEs

Posted by Mark Thomas <ma...@apache.org>.
On 14 July 2014 21:58:22 CEST, Christopher Schultz <ch...@christopherschultz.net> wrote:
>On 7/14/14, 12:18 PM, Mark Thomas wrote:

>>> 2. If the exception is VirtualMachineError, it gets re-thrown with
>>> no log. This skips the "log" call in the above code and so the only
>>> log will come from the VM's "unhandled exception" logger which may
>>> not go where you expect it to go.
>> 
>> It goes to stderr unless they system admin has redirected it and if
>> they have, they should know where to look for it. Further, they
>should
>> also be monitoring it.
>
>True, but the background thread does die.

So? The VM is now in an unknown and potentially unstable state. Restarting the VM is the only sensible way forward so the fact that the background thread has died and any negative consequences of that are irrelevant.

>>> If we think that StackOverflowError is recoverable, why not
>>> OutOfMemory?
>> 
>> There is no assumption that StackOverflowError is recoverable.
>
>    public static void handleThrowable(Throwable t) {
>        if (t instanceof ThreadDeath) {
>            throw (ThreadDeath) t;
>        }
>        if (t instanceof StackOverflowError) {
>            // Swallow silently - it should be recoverable
>            return;
>        }
>        if (t instanceof VirtualMachineError) {
>            throw (VirtualMachineError) t;
>        }
>        // All other instances of Throwable will be silently swallowed
>    }
>
>As of April 17, 2014, markt disagrees with you (r1588269):

I disagree with him all the time. In this case though I think it is just context and that probably just needs a better comment.

Stack overflow in application code should be recoverable as far as Tomcat is concerned. The implication for the application that triggered it may be more severe. There is no assumption that it is recoverable for the application (hence the ERROR level log) but there is a possibility that it might be recoverable therefore - like just about every other application level error - Tomcat carries on.


>http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/util/ExceptionUtils.java?r1=1588269&r2=1588268&pathrev=1588269
>
>> That is why it is logged at ERROR level.
>
>You are right, StackOverflow gets logged, but things like OOME are not
>-- they are left to the unhandled exception handler. But they do
>take-down the background thread when they do.

Other VM errors are assumed to be fatal. Hence I don't care if the background processing thread dies in this case.


>> It is guaranteed that only that
>> thread was affected so you know (from the stack trace) exactly what
>> failed where and you can also determine how bad things are and opt to
>> restart Tomcat if necessary.
>
>For OOME, if you are using sessions you will need to schedule a restart
>pretty much immediately, since they will never die.
>
>> With respect to OOME how, exactly, do you propose to differentiate
>> between a "recoverable" OOME and a non-recoverable one?
>
>If the admin is monitoring (which we are), then they can make the
>determination. The current situation is that Tomcat needs to be bounced
>regardless of the severity of the OOME, because the background thread
>stops.

I disagree. You have no way of knowing the state of the VM after a OOME. You can hope the VM is OK but the only safe option is a restart.

>>> What about other VirtualMachineErrors?
>> 
>> The current position is that they are all non-recoverable. If it can
>> be demonstrated that more of them should be treated like
>StackOverflow
>> then we can do so.
>
>Okay. I think OOME should be treated similarly to StackOverflowError.

I strongly disagree. A stack overflow is guaranteed to only affect the thread that reports the error. It is my understanding that no such guarantee exists for an OOME. I'm prepared to be convinced otherwise (eg by something in one of the Java specs) but until that happens I am -1 on treating OOME like a stack overflow.

I'll add that if it can be demonstrated that any of the other virtual machine errors only affect the current thread and do not indicate something is fundamentally broken in the VM then I'd be happy to treat them the same way as a stack overflow. (Given what virtual machine error is used for I think this is unlikely).

I'll also add that if the current blanket 'allowing' of stack overflow can be shown to be harmful in some cases then I'd be in favour of changing that handling to not re throw the stack overflow only in those cases where it is known to be safe to do so.

Mark



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: ContainerBackgroundProcessor and compounding OOMEs

Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-07-14 23:58 GMT+04:00 Christopher Schultz <ch...@christopherschultz.net>:
> Mark,
>
> On 7/14/14, 12:18 PM, Mark Thomas wrote:
>>> ExceptionUtils.handleException checks for two things that I think
>>> we might want to change:
>>>
>>> 1. If the exception is StackOverflowError, it silently ignores the
>>> error and continues. I think we should at least log something,
>>> probably at WARN level.
>>
>> No it doesn't. Go and read the code again. The exception is logged at
>> ERROR level.
>
> Sorry, yes: the StackOverflow is logged.
>

I filed
https://issues.apache.org/bugzilla/show_bug.cgi?id=56724


Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: ContainerBackgroundProcessor and compounding OOMEs

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,

On 7/14/14, 12:18 PM, Mark Thomas wrote:
>> ExceptionUtils.handleException checks for two things that I think
>> we might want to change:
>>
>> 1. If the exception is StackOverflowError, it silently ignores the
>> error and continues. I think we should at least log something,
>> probably at WARN level.
> 
> No it doesn't. Go and read the code again. The exception is logged at
> ERROR level.

Sorry, yes: the StackOverflow is logged.

>> 2. If the exception is VirtualMachineError, it gets re-thrown with
>> no log. This skips the "log" call in the above code and so the only
>> log will come from the VM's "unhandled exception" logger which may
>> not go where you expect it to go.
> 
> It goes to stderr unless they system admin has redirected it and if
> they have, they should know where to look for it. Further, they should
> also be monitoring it.

True, but the background thread does die.

>> The exception propagates, and the thread's run() method finishes
>> (escapes due to uncaught exception). After that, regardless of the
>> recoverability of the situation (OOME), the background processor
>> will not run and therefore no auto-reload applications will 
>> auto-reload, no sessions will ever die, etc.
>>
>> If we think that StackOverflowError is recoverable, why not
>> OutOfMemory?
> 
> There is no assumption that StackOverflowError is recoverable.

    public static void handleThrowable(Throwable t) {
        if (t instanceof ThreadDeath) {
            throw (ThreadDeath) t;
        }
        if (t instanceof StackOverflowError) {
            // Swallow silently - it should be recoverable
            return;
        }
        if (t instanceof VirtualMachineError) {
            throw (VirtualMachineError) t;
        }
        // All other instances of Throwable will be silently swallowed
    }

As of April 17, 2014, markt disagrees with you (r1588269):
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/util/ExceptionUtils.java?r1=1588269&r2=1588268&pathrev=1588269

> That is why it is logged at ERROR level.

You are right, StackOverflow gets logged, but things like OOME are not
-- they are left to the unhandled exception handler. But they do
take-down the background thread when they do.

> It is guaranteed that only that
> thread was affected so you know (from the stack trace) exactly what
> failed where and you can also determine how bad things are and opt to
> restart Tomcat if necessary.

For OOME, if you are using sessions you will need to schedule a restart
pretty much immediately, since they will never die.

> With respect to OOME how, exactly, do you propose to differentiate
> between a "recoverable" OOME and a non-recoverable one?

If the admin is monitoring (which we are), then they can make the
determination. The current situation is that Tomcat needs to be bounced
regardless of the severity of the OOME, because the background thread stops.

>> What about other VirtualMachineErrors?
> 
> The current position is that they are all non-recoverable. If it can
> be demonstrated that more of them should be treated like StackOverflow
> then we can do so.

Okay. I think OOME should be treated similarly to StackOverflowError.

-chris


Re: ContainerBackgroundProcessor and compounding OOMEs

Posted by Mark Thomas <ma...@apache.org>.
On 14/07/2014 17:01, Christopher Schultz wrote:
> All,
> 
> It seems that ContainerBackgroundProcessor can die and end up
> silently destabilizing things.

It is only silent if the system admin has done something stupid like
redirecting stderr to /dev/null. Frankly, if they do that they deserve
everything they get.

<snip/>

> That means that even if the OOME was transient and the JVM could
> recover from the failure,

I'd argue that an OOME is never recoverable. Once it occurs you have
no idea what allocations may have failed. Even if the thread where the
error is reported can recover from the OOME, you have no way of
determining if any other threads were affected or how they were affected.

> the background processor thread is dead and things like sessions
> will pile up until memory is truly exhausted.
> 
> The problem is in the code for the runnable method. Somewhat
> simplified, it's just a loop to do stuff:
> 
> while(!done) { try { sleep(); processChildren(); } catch (Throwable
> t) { ExceptionUtils.handleException(t); log("error", t); } }
> 
> Although the stack trace doesn't show it, the above error clearly 
> occurred in processChildren().
> 
> ExceptionUtils.handleException checks for two things that I think
> we might want to change:
> 
> 1. If the exception is StackOverflowError, it silently ignores the
> error and continues. I think we should at least log something,
> probably at WARN level.

No it doesn't. Go and read the code again. The exception is logged at
ERROR level.

> 2. If the exception is VirtualMachineError, it gets re-thrown with
> no log. This skips the "log" call in the above code and so the only
> log will come from the VM's "unhandled exception" logger which may
> not go where you expect it to go.

It goes to stderr unless they system admin has redirected it and if
they have, they should know where to look for it. Further, they should
also be monitoring it.

> The exception propagates, and the thread's run() method finishes
> (escapes due to uncaught exception). After that, regardless of the
> recoverability of the situation (OOME), the background processor
> will not run and therefore no auto-reload applications will 
> auto-reload, no sessions will ever die, etc.
> 
> If we think that StackOverflowError is recoverable, why not
> OutOfMemory?

There is no assumption that StackOverflowError is recoverable. That is
why it is logged at ERROR level. It is guaranteed that only that
thread was affected so you know (from the stack trace) exactly what
failed where and you can also determine how bad things are and opt to
restart Tomcat if necessary.

With respect to OOME how, exactly, do you propose to differentiate
between a "recoverable" OOME and a non-recoverable one?

> What about other VirtualMachineErrors?

The current position is that they are all non-recoverable. If it can
be demonstrated that more of them should be treated like StackOverflow
then we can do so.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org