[OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Phil Race
philip.race at oracle.com
Wed Jan 30 23:05:55 UTC 2019
Sorry .. this got lost ..
I don't understand this line :
253 jobjectArray emptyArray = env->NewObjectArray(1, clazz, NULL);
This is returning an array of length 1 and element 0 is NULL.
I think you want
env->NewObjectArray(0, clazz, NULL); and I don't see why you need to
create it there instead you can just have 304 if (nameArray != NULL) {
305 return nameArray;
306 } else {
307 return env->NewObjectArray(0, clazz, NULL);
308 } 449 if (prevRemotePrinters != null) { shouldn't this be 449 if
(prevRemotePrinters != null && prevRemotePrinters.length > 0) { ? -phil.
On 12/10/18 10:19 PM, Shashidhara Veerabhadraiah wrote:
>
> Hi Phil, I have updated with small code updates:
>
> http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.04/
>
> Thanks and regards,
>
> Shashi
>
> *From:*Shashidhara Veerabhadraiah
> *Sent:* Monday, December 10, 2018 3:19 PM
> *To:* Philip Race <philip.race at oracle.com>
> *Cc:* Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>;
> 2d-dev at openjdk.java.net
> *Subject:* RE: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
> tests after JDK-8153732
>
> Hi Phil, Please find the updated Webrev.
>
> http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.03/
>
> Thanks and regards,
>
> Shashi
>
> *From:*Philip Race
> *Sent:* Friday, December 7, 2018 8:54 PM
> *To:* Shashidhara Veerabhadraiah
> <shashidhara.veerabhadraiah at oracle.com
> <mailto:shashidhara.veerabhadraiah at oracle.com>>
> *Cc:* Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com
> <mailto:prasanta.sadhukhan at oracle.com>>; 2d-dev at openjdk.java.net
> <mailto:2d-dev at openjdk.java.net>
> *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
> tests after JDK-8153732
>
> First off, I think the high order bit is that if a non-null array
> reference is returned there are NO
> null String entries in it. Whether a zero length or null array is
> returned when there are no printers
> is the less important issue.
>
> However an empty array also tells you there are no printers, and you
> are less likely to get an NPE from that.
> It is arguably easier for the caller, if they don't need the extra
> null check first.
> FWIW the javax.print public APIs return zero length arrays instead of
> null and applications seem to survive :-)
>
> I don't know what you mean by :
> > (And anyway we need to handle the first null string reference)?
>
> If you are referring to a small matter of coding in the native layer,
> then that is not an insurmountable problem.
>
> Basically if there are problems getting names or whatever in the
> native layer, handle it THERE, don't make
> everyone else have to deal with it.
>
> One caveat: JNI calls can theoretically fail due to OOME .. in such a
> case we are doomed and
> the main goal is to not crash in native and to obey all JNI rules.
> What is returned in that case
> can be a null array reference and an NPE in a Java-level user of that
> reference in such a case is not a big deal.
>
> -phil
>
> On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote:
>
> Hi Phil, Is it advisable to return zero length array from native?
> The null return from native is telling the caller that there are
> no printers connected to the system. To tell this should we
> allocate a zero length array containing null back to the
> caller(And anyway we need to handle the first null string
> reference)? Handling the null on the caller isn’t an easier option?
>
> Thanks and regards,
>
> Shashi
>
> *From:*Phil Race
> *Sent:* Thursday, December 6, 2018 2:35 AM
> *To:* Shashidhara Veerabhadraiah
> <shashidhara.veerabhadraiah at oracle.com>
> <mailto:shashidhara.veerabhadraiah at oracle.com>; Prasanta Sadhukhan
> <prasanta.sadhukhan at oracle.com>
> <mailto:prasanta.sadhukhan at oracle.com>; 2d-dev at openjdk.java.net
> <mailto:2d-dev at openjdk.java.net>
> *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print
> tests after JDK-8153732
>
> But what I am saying is we should not return a NULL string reference
> from native. You are still allowing that and then having to handle
> it in Java code.
>
> And FWIW you can return a zero length array as well so there
> isn't a NULL array reference to deal with either.
>
> -phil.
>
> On 12/3/18 8:29 AM, Shashidhara Veerabhadraiah wrote:
>
> Hi Phil, There were 2 problems earlier. One is that, from the
> native it is possible to have no printers being associated
> with the system(causing to return null reference) and other
> problem in the implementation was that there may be no network
> printers and still return a valid array reference containing a
> null string. The first problem is fixed by handling null
> references returned from this function and other problem was
> that earlier there were different base conditions, for
> updating the reference and use it to create the printer name
> array which could produce a valid reference pointing to null
> string. Here is the updated Webrev which fixes these problems:
>
> http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.02/
> <http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.02/>
>
> The big problem was that I was not able to reproduce this
> problem neither at my end nor at the systems used for PIT
> testing. Only Sergey had produced this NPE till now.
>
> Thanks and regards,
>
> Shashi
>
> *From:*Phil Race
> *Sent:* Wednesday, November 28, 2018 11:19 PM
> *To:* Shashidhara Veerabhadraiah
> <shashidhara.veerabhadraiah at oracle.com>
> <mailto:shashidhara.veerabhadraiah at oracle.com>; Prasanta
> Sadhukhan <prasanta.sadhukhan at oracle.com>
> <mailto:prasanta.sadhukhan at oracle.com>;
> 2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
> *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the
> print tests after JDK-8153732
>
> I am not understanding you. I thought the problem to be we got
> an array of (say) 3 values
> (ie printer names) returned from native where some or all of
> the *values* were NULL.
> And I am saying we should in such a case in the native code,
> before returning,
> remove from the returned array all values that are NULL.
> That could result in an empty (zero length) array being
> returned from native but
> no null names ..
>
> -phil.
>
> On 11/26/18 10:23 PM, Shashidhara Veerabhadraiah wrote:
>
> The windows function EnumPrinters() returns a value that
> could be zero or greater. If it is zero we have no other
> option but to return null from the function. I do not
> think there is a way where we can always make sure we get
> a non-zero value considering the various system scenarios.
> So we should handle the null return values as well in the
> caller of this function I think.
>
> Here is the updated Webrev for test fix:
>
> http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.01/
> <http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.01/>
>
> Thanks and regards,
> Shashi
>
> *From:*Phil Race
> *Sent:* Tuesday, November 27, 2018 1:52 AM
> *To:* Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>
> <mailto:prasanta.sadhukhan at oracle.com>; Shashidhara
> Veerabhadraiah <shashidhara.veerabhadraiah at oracle.com>
> <mailto:shashidhara.veerabhadraiah at oracle.com>;
> 2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
> *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in
> the print tests after JDK-8153732
>
> [Removed swing-dev as this as nothing to do with swing].
>
> You mention in the bug eval that you don't need a new test
> because this
> is already covered by the test for 8153732. If that is the
> case then this
> bugid should be added to that test.
> Although it also looks like there are plenty of tests that
> provoke this ..
> if all you need is a system without any printers then this
> is a serious regression.
>
> I am not sure I am following why doCompare() is the place
> to fix this.
> If getRemotePrinterNames() is returning NULL strings, then
> maybe it should not !
>
> I suggest to fix it there.
>
> -phil.
>
>
>
>
> On 11/26/18 7:51 AM, Prasanta Sadhukhan wrote:
>
> I am not against doCompare() changes. I am just saying
> run() changes are not needed.
>
> Regards
> Prasanta
>
> On 26-Nov-18 9:15 PM, Shashidhara Veerabhadraiah wrote:
>
> There is a possible case and that is the reason
> for this fix. The possible states for
> prevRemotePinters and currentRemotePrinters are
> null and valid values and they can reach those
> states at any time either because of network
> disconnect or any other OS related changes. With
> that in mind, this fix tries to eliminate the
> possible NPE cases.
>
> Thanks and regards,
>
> Shashi
>
> *From:*Prasanta Sadhukhan
> *Sent:* Monday, November 26, 2018 8:01 PM
> *To:* Shashidhara Veerabhadraiah
> <shashidhara.veerabhadraiah at oracle.com>
> <mailto:shashidhara.veerabhadraiah at oracle.com>;
> swing-dev at openjdk.java.net
> <mailto:swing-dev at openjdk.java.net>;
> 2d-dev at openjdk.java.net
> <mailto:2d-dev at openjdk.java.net>
> *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202:
> NPE in the print tests after JDK-8153732
>
> On 26-Nov-18 6:51 PM,
> shashidhara.veerabhadraiah at oracle.com
> <mailto:shashidhara.veerabhadraiah at oracle.com> wrote:
>
> Hi Prasanta, I think we should not create a
> behavior across the functions. doCompare()
> does only the comparison and it may be used
> for other purposes and is complete with
> respect to the comparison functionality.
>
> run() function has a different behavior as it
> needs to populate the prevRemotePrinters and
> then the currentRemotePrinters and then use
> the comparison functionality. I think this is
> a good way to do.
>
> Even without the if-else check, it does populates
> the prevRemotePrinters, no?
> if "prevRemotePrinters" is null and
> currentRemotePrinters is null, then no need to
> update. If currentRemotePrinters is null, then
> what is the point of using
> getRemotePrintersNames() to update
> prevRemotePrinters as currentRemotePrinters is
> also obtained from getRemotePrintersNames!!
> If "prevRemotePrinters" is null and
> currentRemotePrinters is not null, then
> doCompare() called from run() will be true and
> prevRemotePrinters will be populated with
> currentRemotePrinters.
> If "prevRemotePrinters" is not null and
> currentRemotePrinters is null, then also
> prevRemotePrinters will be populated with
> currentRemotePrinters.
>
> Regards
> Prasanta
>
>
>
>
> Thanks and regards,
>
> Shashi
>
> On 26/11/18 6:03 PM, Prasanta Sadhukhan wrote:
>
> Hi Shashi,
>
> I think l437 check of if-else *if
> (prevRemotePrinters != null) {*
>
> is not required. prevRemotePrinters null
> check is addressed in str1==null case in
> doCompare().
> If prevRemotePrinters is null and
> currentRemotePrinters is not null, then
> you update prevRemotePrinters to
> currentRemotePrinters as per l415 where
> doCompare returns true.
> Also, If prevRemotePrinters is not null
> and currentRemotePrinters is null, then
> also you update prevRemotePrinters to
> currentRemotePrinters which is the output
> of getRemotePrintersNames().
>
> Regards
> Prasanta
>
>
>
>
> On 26-Nov-18 2:33 PM, Shashidhara
> Veerabhadraiah wrote:
>
> Hi All, Please review a NPE fix for
> the below bug.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8212202
>
> Webrev:
> http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.00/
> <http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.00/>
>
> Function getRemotePrintersNames() may
> return null values and hence they need
> to be handled from the caller of that
> function which was missing earlier.
> This fix handles the null return
> values of the said function.
>
> Thanks and regards,
>
> Shashi
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20190130/ec070c5d/attachment-0001.html>
More information about the 2d-dev
mailing list