RFR 8158023: SocketExceptions contain too little information sometimes
Roger Riggs
Roger.Riggs at Oracle.com
Wed Jun 29 21:22:04 UTC 2016
Hi Christph,
fine, push when ready.
On 6/29/2016 5:18 PM, Langer, Christoph wrote:
> Hi Roger,
>
> thanks for reviewing.
>
> As for:
>> jni_util.c: line 216:
> There I don't create an extra String but the Exception Object to throw, similar to the old function JNU_ThrowByNameWithLastError.
I misread line 216, thinking the exception class was fixed.
>
>> jni_util.h:
>>
>> line 117-119, The original comment was just as informative as the
> I think you are right - I will restore the old comment.
>
> If no objections I consider this reviewed and will push it tomorrow with the reverted comment lines.
Yep,
Thanks, Roger
>
> Thanks
> Christoph
>
>> -----Original Message-----
>> From: nio-dev [mailto:nio-dev-bounces at openjdk.java.net] On Behalf Of Roger
>> Riggs
>> Sent: Mittwoch, 29. Juni 2016 20:20
>> To: nio-dev at openjdk.java.net
>> Subject: Re: RFR 8158023: SocketExceptions contain too little information
>> sometimes
>>
>> Hi Christoph,
>>
>> Looking good, its unfortunate that the handling of mixed platform and
>> utf string require jni up calls to invoke java methods.
>>
>> jni_util.c: line 216:
>>
>> You should not need to create an extra string; the string s is
>> non-null and ready to use.
>>
>> jni_util.h:
>>
>> line 117-119, The original comment was just as informative as the
>> replacement.
>>
>> The rest looks fine.
>>
>> Roger
>>
>> On 6/28/16 4:45 PM, Langer, Christoph wrote:
>>> Hi Paul,
>>>
>>> Ok, you kind of got me convinced and it is also a quite simple
>>> modification. I changed from “java.net.SocketException: ioctl
>>> SIOCGSIZIFCONF failed: Bad file number” to “java.net.SocketException:
>>> Bad file number (ioctl SIOCGSIZIFCONF failed)” like you suggested.
>>>
>>> The update is in place:
>>> http://cr.openjdk.java.net/~clanger/webrevs/8158023.3/
>>> <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.3/>
>>>
>>> Now I finally need a review…
>>>
>>> Best regards
>>>
>>> Christoph
>>>
>>> *From:*Paul Benedict [mailto:pbenedict at apache.org]
>>> *Sent:* Montag, 27. Juni 2016 18:15
>>> *To:* Langer, Christoph <christoph.langer at sap.com>
>>> *Cc:* Kenji Kazumura <kzr at jp.fujitsu.com>; Chris Hegarty
>>> <chris.hegarty at oracle.com>; nio-dev at openjdk.java.net;
>>> core-libs-dev at openjdk.java.net; net-dev at openjdk.java.net
>>> *Subject:* Re: RFR 8158023: SocketExceptions contain too little
>>> information sometimes
>>>
>>> Christoph, I didn't understand your explanation on your message
>>> preference. Typically root cause information is printed last, not
>>> first. Another reason not to change the ordering of the exception
>>> message is that applications may be looking at existing strings. For
>>> this example, if I may presume "Bad file number" is an existing
>>> message, I would defer to the possibility applications may be exist
>>> that test for that message condition.
>>>
>>>
>>> Cheers,
>>> Paul
>>>
>>> On Mon, Jun 27, 2016 at 2:42 AM, Langer, Christoph
>>> <christoph.langer at sap.com <mailto:christoph.langer at sap.com>> wrote:
>>>
>>> Hi,
>>>
>>> eventually here is the - hopefully final - version of this fix:
>>> http://cr.openjdk.java.net/~clanger/webrevs/8158023.3/
>>> <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.3/>
>>>
>>> Now I leave JNU_ThrowByNameWithLastError untouched and I've also
>>> added conversion to the new function
>>> JNU_ThrowByNameWithMessageAndLastError. I've replaced
>>> JNU_ThrowByNameWithLastError with
>>> JNU_ThrowByNameWithMessageAndLastError in the java/net coding
>>> where I think it is appropriate (mostly in occasions when a
>>> SocketException is thrown kind of generically). @Paul: thanks for
>>> your suggestion regarding the output format but I would still
>>> prefer an output like "java.net.SocketException: ioctl
>>> SIOCGSIZIFCONF failed: Bad file number" vs. "
>>> java.net.SocketException: Bad file number (ioctl SIOCGSIZIFCONF
>>> failed)" as I'm always handing down a message to the new throw
>>> routine.
>>>
>>> Hoping for a review :)
>>>
>>> Best regards
>>> Christoph
>>>
>>> > -----Original Message-----
>>> > From: Kenji Kazumura [mailto:kzr at jp.fujitsu.com]
>>> > Sent: Mittwoch, 8. Juni 2016 02:51
>>> > To: Langer, Christoph <christoph.langer at sap.com>
>>> > Cc: net-dev at openjdk.java.net <mailto:net-dev at openjdk.java.net>;
>>> nio-dev at openjdk.java.net <mailto:nio-dev at openjdk.java.net>; core-libs-
>>> > dev at openjdk.java.net <mailto:dev at openjdk.java.net>
>>> > Subject: Re: RFR 8158023: SocketExceptions contain too little
>>> information
>>> > sometimes
>>> >
>>> > Christoph,
>>> >
>>> > You should not remove conversion codes (platform string to Java
>>> String)
>>> > at JNU_ThrowByNameWithLastError,
>>> > and you have to add conversion codes at
>>> > JNU_ThrowByNameWithMessageAndLastError.
>>> >
>>> > It seems that you assume strerror returns only ascii characters,
>>> but actually
>>> > not.
>>> > It depends on the locale of your environment where java programs
>>> runs.
>>> >
>>> >
>>> > -Kenji Kazumura
>>> >
>>> >
>>> > In message
>>> >
>> <decc19cdab854bbeac7126cb8e236f1e at DEWDFE13DE11.global.corp.sap
>> <mailto:decc19cdab854bbeac7126cb8e236f1e at DEWDFE13DE11.global.corp.sa
>> p>>
>>> > RFR 8158023: SocketExceptions contain too little information
>>> sometimes
>>> > "Langer, Christoph"
>>> <<mailto:christoph.langer at sap.com>christoph.langer at sap.com> wrote:
>>> >
>>> > > Hi all,
>>> > >
>>> > > please review the following change:
>>> > > Webrev:
>>>
>> <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.1/>http://cr.openjdk.
>> java.net/~clanger/webrevs/8158023.1/
>>> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8158023
>>> > >
>>> > > During error analysis I stumbled over a place where I
>>> encountered a
>>> > SocketException which was thrown along with some strerror
>>> information as
>>> > message. I found it hard to find the originating code spot with
>>> that information.
>>> > >
>>> > > So I looked at the places where we throw exceptions, namely
>>> JNU_Throw...
>>> > and NET_Throw... functions and came up with the following
>>> enhancement:
>>> > > - NET_ThrowByNameWithLastError can go completely as it does
>>> not provide
>>> > any benefit over JNU_ThrowByNameWithLastError.
>>> > > - JNU_ThrowByNameWithLastError can be cleaned up.
>>> > >
>>> > > - I added JNU_ThrowByNameWithMessageAndLastError to print out
>>> a string
>>> > like message + ": " + last error.
>>> > >
>>> > > - I went over all places where NET_ThrowByNameWithLastError is
>>> used and
>>> > replaced it appropriately.
>>> > >
>>> > > Do you think this change is desirable/possible?
>>> > >
>>> > > Though it's mainly a net topic, I'm posting it to nio-dev and
>>> core-libs-dev as
>>> > well as JNU_Throw... code affects all.
>>> > >
>>> > > Best regards
>>> > > Christoph
>>> > >
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20160629/1b03b3b6/attachment-0001.html>
More information about the nio-dev
mailing list