tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: Recent tcnative null-dereference with 8.0.0-RC3 and 7.0.45 [tcnative-1.dll+0x7e23]
Date Tue, 01 Oct 2013 14:15:02 GMT
Mladen and Rainer,

On 10/1/13 2:59 AM, Rainer Jung wrote:
> On 01.10.2013 06:53, Mladen Turk wrote:
>> On 09/30/2013 08:50 PM, Christopher Schultz wrote:
>>> Mladen,
>>>
>>> Unless there is significant objection,
>>
>> Yes there is.
>> The transition from  Java to JNI costs 10 times then
>> then a simple 'if (socket == OL)' in java

Okay, so we can add those in Java as well. I still think that tcnative
should be coded defensively.

>>> I'd like to add such NULL checks
>>> to limit JVM crashes in cases where the Java code isn't absolutely
>>> perfect (with sincere apologies to all who manage the socket-management
>>> code in Tomcat).
>>>
>>
>> -1. You are just hiding the reason for crash.

I disagree: the reason for the crash can still be reported. It just
won't be reported with a JVM crash: instead, there will be an exception.
We can even rig it to report the line number in the native method if you
want. Wouldn't that be better than bringing-down the whole JVM? I'd
rather have a complete lock-up of the JVM with a handful of exceptions
explaining the problem than having to examine a core file to figure out
what happened. I'm sure that's true of most of our users, too.

> I hate these crashes too, but I'm agreeing with Mladen here, especially
> if the native call does not return an error on such unexpected input
> data and the error is not handled in Java land (logging, maybe more).

I wasn't suggesting that the Java component take no action, here: I was
just suggesting that a exception (even if fatal) is better than a JVM
crash. I don't see why both components can't check their own stuff.

> In cases where the crashes indicate that there was a data corruption
> before, like a synchronization issue, we need to get aware of that.
> Otherwise we might serve invalid results.
> 
> It would be nice though, if there were a simple validity check in Java
> land, so that we could e.g. log an error if we encounter invalid data
> and handle the error accordingly.

We can add those, too. I'm trying to understand which data are intended
to be native pointers so we can do that kind of thing. I'm happy to add
zero-checks in Java as well as NULL checks in the native code. I just
didn't want to go around changing things in Java-land if Mark is doing
the same: his changes will likely be more meaningful than anything I do.

-chris


Mime
View raw message