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 Mandy

On 20/03/17 20:40, Mandy Chung wrote:
> Hi Daniel,
>
> Have you considered default to the unnamed module when there is no caller frame on the stack?
>
> I don’t think we should make these APIs not callable from JNI attached thread even though very rarely be called from JNI (not sure any report on the current behavior throwing NPE).

I don't think we should make assumption or guesses.
As Alan stated: "If there's no caller then you can't make any
assumptions as to who the caller is. Having these methods assume
java.base or the unnamed module of some class loader doesn't seem right
as there just isn't enough context."

There are at least two possibilities if someone wants to get a
System.Logger from a JNI attached thread:

   1. Use an auxilliary class to indicate the implicit caller (that
      will be the auxilliary class module)

   2. Call LoggerFinder.getLoggerFinder().getLogger and explicitly supply
      a module

IMO calling directly System.getLogger is not appropriate in this case.

best,

-- daniel

>
> Mandy
>
>> On Mar 20, 2017, at 5:08 AM, Daniel Fuchs <daniel.fuchs at oracle.com> 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
>