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:47:26 GMT
Mladen,

On 10/1/13 10:39 AM, Mladen Turk wrote:
> On 10/01/2013 04:15 PM, Christopher Schultz wrote:
>> Mladen and Rainer,
>>>>
>>>> -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.
> 
> I disagree on your disagreement :)

;)

Just to be sure: I'm not arguing against fixing the Java code, or
providing checks in the Java code to avoid SIGSEGVs. I'm just saying
that native code should be as safe as reasonably possible. NULL-checks
are cheap, too, so I don't think there's any reason to avoid them in
native code.

> This can be easily done in Java with much cheaper computing cost.

Agreed. Feel free to avoid native calls if you know the data is bad. But
we can't force people to use specific versions of Tomcat and tcnative
together (though obviously, newer versions of Tomcat can refuse to load
older versions of tcnative). Someone pointed out in Bugzilla (though it
mostly fell on deaf ears) that tcnative isn't used exclusively for
Tomcat: some folks are using it as a convenient library to access APR
and other native capabilities in a Java wrapper.

> If you suspect the data fed to native call can be faulty, well check it
> and throw java exception instead calling native, and distinguishing
> between valid
> and error return values from native and then still have a code which will
> pass/throw.

I was suggesting throwing an exception directly from native code, not
trying to return a status code and then throwing from Java-land.

> I know it requires a different thinking then average library,
> but it's not an average library. It's wrapper for APR and APR expects you
> provide valid data.
> 
> Even checking in native won't solve crashes. You can fed a long (pointer)
> which is in zombie state (closed but not null) and you'll have a memory
> location which will pass null check but can still crash or even corrupt
> other memory location if it was reused by another alloc.

Sure, but a NULL-check (whether in Java or C) isn't going to catch that
either way, and nothing we've been discussing here will have any bearing
on that situation: it will still fail, it will likely still crash or
behave strangely and ...

> Much harder to track down.

... it will be hard to track down. Adding NULL-checks will not
complicate any of that. It will not mask any of that. It will make it no
more difficult to track-down or debug. It will only prevent the JVM
going down when a pointer ends up being NULL, which is the only reason
I'm suggesting it.

Given the two -1s and lack of any +1s on this suggestion, I won't be
committing anything, but I still think it's a worthwhile endeavor
because it's so easy to do and protects us from such disastrous results.

-chris


Mime
View raw message