RFR: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack.

[Daniel Fuchs]

Hi David,

On 20/03/2017 12:16, David Holmes wrote:
> Sorry Daniel but I don't understand how calling a public method via JNI
> can be deemed an illegal call ???

In the case of these methods, it's only illegal if there's no
java frame on the stack as the caller's module cannot be determined.
It's perfectly OK to call them from JNI has long as there's
some Java frame (main() or Thread::run) higher up.

> This seems to be a hole in the notion
> of "caller sensitive" to me.

I'd rather say it's a hole in how such a situation is handled.

> Also see discussion re: "RFR [9]: 8177036: Class.checkMemberAccess
> throws NPE when calling Class methods via JNI"

Yes - I am aware of this and this is what triggered my
proposed patch for the few places where we use @CallerSensitive
in logging.

An application that would need can to call these logging methods
from such a context (where no Java frame is on the stack) can
always insert its own wrapper (e.g. a call these methods from an
application class and call that application class from JNI instead).

best regards,

-- daniel

>
> David
> -----
>
> On 20/03/2017 10:08 PM, Daniel Fuchs wrote:
>> Hi,
>>
>> Please find below a patch for:
>>
>> 8177136: Caller sensitive methods Logger.getLogger,
>> Logger.getAnonymousLogger, and System.getLogger should throw
>> IllegalCallerException if there is no caller on the stack.
>> https://bugs.openjdk.java.net/browse/JDK-8177136
>>
>> Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger,
>> and System.getLogger currently throw an undocumented
>> NullPointerException if they are called from JNI and there is no
>> Java frame on the stack.
>>
>> Throwing NullPointerException is confusing and makes it look like there
>> is a bug in the implementation.
>> In truth, these method are @CallerSensitive, and therefore must not
>> be called in a context where the caller cannot be determined.
>> Therefore, the right thing to do is to throw IllegalCallerException
>> and document this.
>>
>> webrev:
>> http://cr.openjdk.java.net/~dfuchs/webrev_8177136/webrev.00/
>>
>> As per Rampdown Phase 2 Process [1] I'd also like to get
>> confirmation that this is a reasonable proposal to fix in 9.
>> This fix just transmutes a NullPointerException (which should
>> never happen at this point in regular usage of the API) into an
>> IllegalCallerException which will help diagnosing the fact
>> that the API is called from a context where it's not supposed
>> to be used. The risk of fixing should therefore be very limited.
>>
>> best regards,
>>
>> -- daniel
>>
>> [1] Rampdown Phase 2 Process
>> http://mail.openjdk.java.net/pipermail/jdk9-dev/2017-March/005666.html