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

[David Holmes]

On 20/03/2017 10:29 PM, Daniel Fuchs wrote:
> 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.

What about those API's says there has to be a Java frame higher up. Why 
can't an attached thread request a reference to the logger?

David
-----

>> 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
>