https://bugs.openjdk.java.net/browse/JDK-8201545
-Jaikiran
# HG changeset patch
# User Jaikiran Pai
# Date 1524215697 -19800
# Fri Apr 20 14:44:57 2018 +0530
# Node ID fbafd0fdcb0033f7d6b65b5686b26e8f23a46c5b
# Parent dd5db907ab7e09c87b4dd246f960d97e33fe9c7a
JDK-8201545 Clarify the return val
.
On 20/04/18 11:08, Jaikiran Pai wrote:
Hi,
The attached patch addresses the issue noted in [1], by updating the
javadoc of InetAddress.getByName and InetAddress.getAllByName to
clarify that these methods return a loopback address, if the host
parameter is an empty string (same behaviour as host
Hello,
I've been experimenting with the new HttpClient API that's part of Java
now. I am using the latest EA build 16 of Java 11 from here[1]. I'm
still in the early stages of integrating this into one of the libraries
where we use HTTP clients.
Based on my usage so far, I have some minor
f
Hi,
Attached is a trivial patch which fixes a typo in the javadoc of
java.net.http.HttpClient class.
I've a signed and approved OCA, but will need a sponsor for this patch.
-Jaikiran
# HG changeset patch
# User Jaikiran Pai
# Date 1527920758 -19800
# Sat Jun 02 11:55:58 2018
You are right. That didn't occur to me. So yes, clearly, adding those 2 new
methods won't add value.
-Jaikiran
On Sunday, June 3, 2018, Pavel Rappo wrote:
>
>> On 2 Jun 2018, at 07:22, Jaikiran Pai wrote:
>>
>> Some of the places where we parse the (response) hea
Consider the following code:
public class HttpClientTest {
public static void main(final String[] args) {
final HttpClient httpClient = HttpClient.newBuilder().build();
final String targetURL = "http://unknown.host.foo.bar.com.nowhere";;
final HttpRequest request =
Ht
Hi Michael,
I'm not a reviewer. I just checked the webrev and saw this change:
--- old/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java
2018-06-06 08:34:38.0 +0100
+++ new/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java
2018-06-06 08:34:3
x27;t be overridden programatically (since that can potentially
behave differentlydepending on when the SocketExceptions class gets
initialized)?
-Jaikiran
On 08/06/18 12:05 PM, Jaikiran Pai wrote:
Hi Michael,
I'm not a reviewer. I just checked the webrev and saw this change:
-
Hi Chris,
On 06/06/18 5:04 PM, Chris Hegarty wrote:
Jaikiran,
3. Is the latest API javadoc this one[2]? Based on what I read and experimented
so far, I don't see a way to set a value for User-Agent at the HttpClient
level. Unless of course, I'm expected to keep setting it on each request th
Any thoughts?
-Jaikiran
On Sunday, June 3, 2018, Jaikiran Pai wrote:
> Consider the following code:
>
> public class HttpClientTest {
> public static void main(final String[] args) {
> final HttpClient httpClient = HttpClient.newBuilder().build();
> fina
Hi Chris,
On 12/06/18 8:05 PM, Chris Hegarty wrote:
I filed the following JIRA to track this issue:
https://bugs.openjdk.java.net/browse/JDK-8204864
I think that the most appropriate course of action is to consistently throw
the IOException subtype, ConnectionException ( with the original
I couldn't locate this bug in the JIRA nor the bugs.java.net, to see if
it's acknowledged as an issue. So FWIW - I can reproduce this even on
MacOS (so it isn't just specific to Windows OS). This is the code:
import java.net.URL;
import java.io.InputStream;
public class CertTest {
public s
I'm testing/integrating the latest OpenJDK upstream HttpClient
implementation with one of the libraries I have. Thanks for fixing some
of the issues I (and others) had raised before!
Before the introduction of this new API, the HttpURLConnection (which
acted as a basic minimal client for HTTP)
Thanks Daniel.
-Jaikiran
On 25/06/18 10:49 PM, Daniel Fuchs wrote:
Hi Jaikiran,
Yes - it does support preemptive auth, but only for basic
authentication since only basic is supported out of the
box.
best regards,
-- daniel
On 25/06/2018 12:46, Jaikiran Pai wrote:
I'm testing/integr
java.net.http/jdk.internal.net.http.AsyncTriggerEvent.handle(AsyncTriggerEvent.java:54)
at
java.net.http/jdk.internal.net.http.HttpClientImpl$SelectorManager.run(HttpClientImpl.java:796)
# HG changeset patch
# User Jaikiran Pai
# Date 1530018198 -19800
# Tue Jun 26 18:33:18 2018
}
} finally {
server.stop(0);
-Jaikiran
On 26/06/18 7:12 PM, Jaikiran Pai wrote:
In my random experimentation with the new HttpClient API usage, I have
ended up running into an odd and hard to decipher exception when
dealing with PUT requests. I am noticing that if I issue multiple
Thanks Chris!
-Jaikiran
On 26/06/18 7:34 PM, Chris Hegarty wrote:
Hi Jaikiran,
I filed the following JIRA issue to track this:
https://bugs.openjdk.java.net/browse/JDK-8205684
Thanks for the test and initial investigation. We’ll look into it.
-Chris.
On 26 Jun 2018, at 14:52, Jaikiran Pai
ion I am concerned that I might not
be seeing it)
best regards,
-- daniel
On 26/06/2018 14:52, Jaikiran Pai wrote:
So it looks like there some race condition somewhere in the
HttpClient implementation. I just addeda smalldelay between the PUT
requests, in the test that I sent in the patch and i
Hi Daniel,
On 26/06/18 8:26 PM, Daniel Fuchs wrote:
Hi Jaikiran,
On 26/06/2018 15:41, Jaikiran Pai wrote:
So is this a case of the server implementation misbehaving? I haven't
checked what the expectations are from a spec point of view for
handling such requests on the server. Or is
Hi Chris,
src/java.net.http/share/classes/java/net/http/HttpClient.java
+ * In the case where a new connection needs to be
established, if
+ * the connection cannot be established within the given {@code
+ * duration}, then an {@link HttpConnectTimeoutException} is thrown
Thanks for the updates, Chris. This looks fine to me (I'm not a reviewer).
-Jaikiran
On 07/08/18 3:22 PM, Chris Hegarty wrote:
>> On 4 Aug 2018, at 14:08, Jaikiran Pai wrote:
>>
>> ...
>>
>> Do you think this can be reworded a bit? Although I understand wha
ts: passed: 190
Could I please get a review of this patch and someone to sponsor it?
[1] https://tools.ietf.org/html/rfc2616#section-6.1.1
-Jaikiran
# HG changeset patch
# User Jaikiran Pai
# Date 1560302554 -19800
# Wed Jun 12 06:52:34 2019 +0530
# Node ID fd96785afb763c7daaf189bf9ce037
the patch, I can do
that too.
Thank you for your help so far.
-Jaikiran
>
> best regards,
>
> -- daniel
>
> [1] http://openjdk.java.net/census#jpai
> [2] http://cr.openjdk.java.net/~jpai/
> [3] http://openjdk.java.net/projects/code-tools/
> [4] http://openjdk.java.ne
Hi Daniel,
On 12/06/19 7:30 PM, Daniel Fuchs wrote:
> Hi Jaikiran,
>
> On 12/06/2019 13:41, Jaikiran Pai wrote:
>> I decided to give it a try and host it on cr.openjdk.java.net. I have
>> now published the webrev for this patch, here
>> http://cr.openjdk.java.
Hello Daniel,
On 13/06/19 7:11 PM, Daniel Fuchs wrote:
> Hi Jaikiran,
>
> On 12/06/2019 16:01, Jaikiran Pai wrote:
>> The new webrev is now available for review at
>> http://cr.openjdk.java.net/~jpai/8217705/01/webrev/
>
> I'm happy to report that tests have b
Thank you Daniel.
-Jaikiran
On 14/06/19 3:46 PM, Daniel Fuchs wrote:
> Hi Jaikiran,
>
> On 14/06/2019 06:29, Jaikiran Pai wrote:
>> Long story short, I think I have managed to get this right, this time.
>> The updated webrev is here
>> http://cr.openjdk.java.net/~jp
Hello,
Could I please get a review and a sponsor for a patch which fixes an
issue in sun.net.spi.DefaultProxySelector? The patch is available as a
webrev at
http://cr.openjdk.java.net/~jpai/webrev/defaultproxyselector/1/webrev/
The issue has been reported in multiple different JBS issues[1][2][3]
s what users are expecting.
I can update the proposed patch to redo it to catch the IAE and throw
the IOException at the relevant places. Would you also want me to update
any javadoc of the ProxySelector to make it more clear of this (so far)
implicit behaviour? Does that require a CSR?
-Jaikiran
Hello Michael,
On 31/07/19 11:43 AM, Michael McMahon wrote:
> ...
>>>
>>> I wonder if another solution is possible where the IAE is caught
>>> at the appropriate place(s) and converted to an IOException
>>> which is what users are expecting.
>> I can update the proposed patch to redo it to catch t
your help so far on reviewing this.
-Jaikiran
>
> Thanks
> Michael.
>
> On 01/08/2019, 08:03, Jaikiran Pai wrote:
>> Hello Michael,
>>
>> On 31/07/19 11:43 AM, Michael McMahon wrote:
>>> ...
>>>>> I wonder if another solution is possib
Hello Julia,
I'm not a reviewer, but I noticed this change:
src/java.base/share/classes/sun/net/www/MessageHeader.java
public synchronized String toString() {
String result = super.toString() + nkeys + " pairs: ";
+ String reqLine = this.requestLine;
for (int i = 0;
t; gets called which could benefit from the same change. Personally, I
>> think it would be okay
>> to deal with those in a followup issue, but maybe others have another
>> view on that.
>> I can create the CSR for this, but it could be next week before I get
>> around
Can I please get a review and a sponsor for the patch for
https://bugs.openjdk.java.net/browse/JDK-8177389. The patch is available
as a webrev at http://cr.openjdk.java.net/~jpai/webrev/8177389/1/webrev/.
The change that's asked for, in that JBS, is actually trivial. While I
was at it, I also chan
Thank you Michael for reviewing and sponsoring.
-Jaikiran
On 26/08/19 5:02 PM, Michael McMahon wrote:
> Jaikiran,
>
> The CSR was approved, and the change has been pushed.
> Thanks for the contribution.
>
> Michael.
>
> On 22/08/2019, 17:19, Jaikiran Pai wrote:
>
Can I please get a review and a sponsor for a patch which fixes the
issue noted in [1]? The patch is available as a webrev at [2].
This is a newly added testcase as part of a patch that I recently
contributed for [3]. As noted by Daniel in [1], the intermittent failure
is a result of a race condit
Hi Daniel,
On 27/08/19 4:49 PM, Daniel Fuchs wrote:
> Hi Jaikiran,
>
> Looks good.
>
> While you're at it, would you mind using
> jdk.test.lib.net.URIBuilder from the /test/lib
> library instead of composing the URL string by hand?
>
> Something like:
>
> URL targetURL = URIBuilder.newBuilder(
Hi Daniel,
On 27/08/19 5:43 PM, Daniel Fuchs wrote:
> Looks good to me Jaikiran.
>
> Do you want me to sponsor this for you?
Yes, please.
>
> You can count me as Reviewer.
Thank you. I've updated the webrev at the same location[1] to just
include you in the Reviewed-by. No other changes done.
[1
Can I please get a review and a sponsor for the patch for
https://bugs.openjdk.java.net/browse/JDK-8230310? The patch is hosted as
a webrev at [1].
As noted in that JBS issue, this is a follow-up patch to the changes
that were done as part of [2]. During that patch review, it was decided
that rema
Hello Daniel,
On 28/08/19 8:45 PM, Daniel Fuchs wrote:
> Hi Jaikiran,
>
> On 28/08/2019 15:47, Jaikiran Pai wrote:
>> Can I please get a review and a sponsor for the patch for
>> https://bugs.openjdk.java.net/browse/JDK-8230310? The patch is hosted as
>> a webrev at
On 29/08/19 9:12 AM, Jaikiran Pai wrote:
> Hello Daniel,
>
> On 28/08/19 8:45 PM, Daniel Fuchs wrote:
>> Hi Jaikiran,
>>
>> On 28/08/2019 15:47, Jaikiran Pai wrote:
>>> Can I please get a review and a sponsor for the patch for
>>> https://bugs.open
Hello Daniel,
Here's an updated webrev which includes a test case to verify this
change http://cr.openjdk.java.net/~jpai/webrev/8230310/2/webrev/
-Jaikiran
On 29/08/19 9:12 AM, Jaikiran Pai wrote:
> Hello Daniel,
>
> On 28/08/19 8:45 PM, Daniel Fuchs wrote:
>> Hi Jaikiran,
Hello Daniel,
On 29/08/19 2:56 PM, Daniel Fuchs wrote:
> Thanks for the test Jaikiran!
>
> Since an IOException is expected at line 76, I believe
> it would be good for the test to trow an assertion error
> if that doesn't happen.
>
> Maybe simply insert:
> throw new AssertionError("expected ex
e to the Reviewed-by field in the commit log.
Thank you Daniel for reviewing and sponsoring.
-Jaikiran
>
> On 29/08/2019 10:48, Jaikiran Pai wrote:
>>> Maybe simply insert:
>>> throw new AssertionError("expected exception not thrown");
>>> between l
Can I please get a review and a sponsor for a patch for
https://bugs.openjdk.java.net/browse/JDK-8223714?
The patch is available as a webrev at
http://cr.openjdk.java.net/~jpai/webrev/8223714/1/webrev/
This issue and the patch relates solely to a test infrastructure class -
HTTPTestServer and tou
On 30/08/19 5:40 PM, Jaikiran Pai wrote:
> Can I please get a review and a sponsor for a patch for
> https://bugs.openjdk.java.net/browse/JDK-8223714?
>
> The patch is available as a webrev at
> http://cr.openjdk.java.net/~jpai/webrev/8223714/1/webrev/
>
> This issue and the
Hello Daniel,
On 30/08/19 9:14 PM, Daniel Fuchs wrote:
> Hi Jaikiran,
>
> 1075 System.out.println("Server will stop
> accepting connections due to an exception");
> 1076 io.printStackTrace(System.out);
> 1077 break;
>
> Two things here:
>
Hello Vyom,
Thank you for the review. Comments inline.
On 31/08/19 7:05 PM, Vyom Tewari26 wrote:
> Hi Jaikiran,
>
> Code changes look OK to me , couple of minor comments
> 1-> you don't need "this" to access stop(this.stop)
Usage of this.member, is more of a personal practice that I try to
fol
On 02/09/19 9:21 AM, Jaikiran Pai wrote:
>
> Hello Vyom,
>
> Thank you for the review. Comments inline.
>
> On 31/08/19 7:05 PM, Vyom Tewari26 wrote:
>> Hi Jaikiran,
>>
>> Code changes look OK to me , couple of minor comments
>> 1-> you don't
Hello Vyom, Daniel,
On 02/09/19 3:00 PM, Daniel Fuchs wrote:
> Hi,
>
> On 02/09/2019 05:29, Vyom Tewari26 wrote:
>>> 2-> i think super.stop() should be the first line in the
>>> overridden stop method.
>>
>> I had thought of this when I introduced the change. However, I felt
>> tha
Thank you Daniel.
-Jaikiran
On 03/09/19 10:20 PM, Daniel Fuchs wrote:
> Hi Jaikiran,
>
> Tests were successful and I pushed it...
>
> best regards,
>
> -- daniel
>
> On 02/09/2019 12:29, Daniel Fuchs wrote:
>> On 02/09/2019 12:23, Jaikiran Pai wrote:
>>>
Ping. Anyone willing to review and sponsor, please?
-Jaikiran
On 26/08/19 4:56 PM, Jaikiran Pai wrote:
> Can I please get a review and a sponsor for the patch for
> https://bugs.openjdk.java.net/browse/JDK-8177389. The patch is available
> as a webrev at http://cr.openjdk.java.net/~jp
e further than just removing the superfluous
> hyphen but I think that's OK.
> You can count me as Reviewer. If nobody objects I'll hopefully
> push that for you later today.
>
> best regards,
>
> -- daniel
>
> On 04/09/2019 11:55, Jaikiran Pai wrote:
&g
Can I please get a review and a sponsor for a patch which fixes the
issue reported in JDK-8241138[1]?
The patch is hosted as a webrev at [2]. The change in the patch now adds
an explicit equals check for "*" before checking it for various other
possible patterns. The patch also updates an existing
Thank you Chris and Daniel for the reviews and the sponsoring.
-Jaikiran
On 18/03/20 5:33 pm, Daniel Fuchs wrote:
> Hi Jaikiran,
>
> This looks reasonable to me.
> I will happily sponsor it - unless there are other takers.
>
> best regards,
>
> -- daniel
>
> On
There's an issue raised in Quarkus repo[1], where Apache Kafka
(embedded) no longer starts on Java 14. From what I can see the root
cause is this[2].
JDK-8225499[3] changed the implementation (and even the javadoc) of the
InetSocketAddress.toString() which now reads[4]:
"...
If the address is un
Hello Alan,
On 30/03/20 1:31 pm, Alan Bateman wrote:
> On 28/03/2020 06:43, Jaikiran Pai wrote:
>>
>> There's an issue raised in Quarkus repo[1], where Apache Kafka
>> (embedded) no longer starts on Java 14. From what I can see the root
>> cause is this[2].
Hello Chris,
On 30/03/20 1:49 pm, Chris Hegarty wrote:
> Hi Jaikiran,
>
> Thank you for posting this message to net-dev.
>
> TL;DR seems like the specific issue raised against Quarkus has been
> resolved ( by upgrading to a more recent version of ZooKeeper ).
That's correct.
>
>> So this looks l
On 01/04/20 3:49 pm, Julia Boes wrote:
>
> Hi Jaikiran,
>
By the way, keeping aside the breaking nature of this change, the
current javadoc, I think doesn't reflect what the current
implementation returns for unresolved addresses. The javadoc states:
"If the address is unr
Thank you Chris.
-Jaikiran
On 01/04/20 6:40 pm, Chris Hegarty wrote:
>
>
>> On 1 Apr 2020, at 13:48, Jaikiran Pai > <mailto:jai.forums2...@gmail.com>> wrote:
>>>>> ...
>>>>
>>>> Now that you explained it to me, I see what you mean a
The security.properties file in Java 11 (and later) have a new
configuration:
# The categories are:
#
# hostInfo - IOExceptions thrown by java.net.Socket and the socket
types in the
# java.nio.channels package will contain enhanced exception
# message information
#
# The p
Hello Alan,
On 11/06/20 7:18 pm, Alan Bateman wrote:
> On 11/06/2020 14:31, Jaikiran Pai wrote:
>> The security.properties file in Java 11 (and later) have a new
>> configuration:
>>
>> # The categories are:
>> #
>> # hostInfo - IOExceptions thrown by jav
On 11/06/20 7:40 pm, Alan Bateman wrote:
> On 11/06/2020 14:59, Jaikiran Pai wrote:
>> :
>> Is this a valid use case to have this part of the JDK code enhanced to
>> include the additional host/port information (of course when that
>> security property is enabled)?
&
On 11/06/20 7:47 pm, Jaikiran Pai wrote:
>
> I'll open a JBS enhancement in a short while to have this part enhanced
> to report the additional details only when the specific security
> property is enabled.
I've created https://bugs.openjdk.java.net/browse/JDK-8247413 to track this.
-Jaikiran
On Sun, 6 Sep 2020 05:46:42 GMT, Jaikiran Pai wrote:
> Can I please get a review and a sponsor for a fix for the issue reported at
> https://bugs.openjdk.java.net/browse/JDK-8252767?
> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` is
> throwing a `Illega
Can I please get a review and a sponsor for a fix for the issue reported at
https://bugs.openjdk.java.net/browse/JDK-8252767?
As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` is
throwing a `IllegalAccessError` instead
of a `IllegalStateException`. The commit here fixes t
On Sun, 6 Sep 2020 09:05:55 GMT, Alan Bateman wrote:
>> Can I please get a review and a sponsor for a fix for the issue reported at
>> https://bugs.openjdk.java.net/browse/JDK-8252767?
>> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty`
>> is throwing a `IllegalAccessEr
On Sun, 6 Sep 2020 09:11:52 GMT, Jaikiran Pai wrote:
>> test/jdk/sun/net/www/URLConnectionTest.java line 36:
>>
>>> 34: * @run testng URLConnectionTest
>>> 35: */
>>> 36: public class URLConnectionTest {
>>
>> I think it would be better to
on`. The commit here fixes that and includes a test
> which reproduces the issue and verifies
> the fix. Would a CSR be needed for this change?
Jaikiran Pai has refreshed the contents of this pull request, and previous
commits have been removed. The incremental
views will show differences com
on`. The commit here fixes that and includes a test
> which reproduces the issue and verifies
> the fix. Would a CSR be needed for this change?
Jaikiran Pai has refreshed the contents of this pull request, and previous
commits have been removed. The incremental
views will show differences com
On Sun, 6 Sep 2020 10:56:44 GMT, Alan Bateman wrote:
> The URL creation isn't technically right as you can't reliably use a file
> path string as a URL path. Something like the
> following would be more correct:
URL url = Path.of(System.getProperty("java.io.tmpdir")).toUri().toURL();
I've now u
On Mon, 7 Sep 2020 08:33:17 GMT, Chris Hegarty wrote:
>> Jaikiran Pai has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental
>> views will show differences compared to the previous content of the PR. The
>> pull
on`. The commit here fixes that and includes a test
> which reproduces the issue and verifies
> the fix. Would a CSR be needed for this change?
Jaikiran Pai has refreshed the contents of this pull request, and previous
commits have been removed. The incremental
views will show differences com
On Mon, 7 Sep 2020 09:28:39 GMT, Alan Bateman wrote:
> I see Jaikiran has used "/integrate" but I don't think the review is complete
> yet. The change may be trivial but it's
> really important to give others a chance to comment if they wish.
Sorry, I'm new this whole flow. I saw the message fr
On Mon, 7 Sep 2020 09:24:06 GMT, Alan Bateman wrote:
> Given that the test update is significant then it makes me wonder if we
> should just move to TestNG while we're at it.
> It would avoid needing to add expectIllegalStateException.
I wasn't sure if it was OK to do it as part of this change.
On Mon, 7 Sep 2020 09:31:02 GMT, Jaikiran Pai wrote:
>> I see Jaikiran has used "/integrate" but I don't think the review is
>> complete yet. The change may be trivial but it's
>> really important to give others a chance to comment if they wish.
>
>
on`. The commit here fixes that and includes a test
> which reproduces the issue and verifies
> the fix. Would a CSR be needed for this change?
Jaikiran Pai has updated the pull request incrementally with one additional
commit since the last revision:
Convert to TestNG test case
-
On Mon, 7 Sep 2020 09:37:23 GMT, Chris Hegarty wrote:
> @jaikiran No need to be sorry, you are following the process as it is
> outlined. Please go ahead with the suggestion to
> update the test to use TestNG. The current state of the PR should not affect
> updates. BTW AFAIK, you should not ne
on`. The commit here fixes that and includes a test
> which reproduces the issue and verifies
> the fix. Would a CSR be needed for this change?
Jaikiran Pai has updated the pull request incrementally with one additional
commit since the last revision:
Use new TestNG As
On Mon, 7 Sep 2020 11:24:18 GMT, Chris Hegarty wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Convert to TestNG test case
>
> test/jdk/java/net/URLConnection/RequestProperties.java
On Mon, 7 Sep 2020 12:41:49 GMT, Daniel Fuchs wrote:
>> I think this version looks much better. A few nits with naming/style but not
>> worth spending time on.
>
> Hi Jaikiran,
>
> I'll sponsor this for you when it is ready to integrate but I'd like to send
> it to our test system first.
>
>
On Sun, 6 Sep 2020 05:46:42 GMT, Jaikiran Pai wrote:
> Can I please get a review and a sponsor for a fix for the issue reported at
> https://bugs.openjdk.java.net/browse/JDK-8252767?
> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` is
> throwing a `Illega
As noted by the reporter in the comments of
https://bugs.openjdk.java.net/browse/JDK-7146776, the `URLStreamHandler` in its
`getHostAddress` is incorrectly synchronizing on the `URLStreamHandler`
instance, instead of synchronizing on the `URL` instance that is being passed
to that method.
The
On Sat, 16 Jan 2021 19:05:46 GMT, Alan Bateman wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Javadoc update
>
> src/java.base/share/classes/java/net/URL.java line 817:
>
>>
ich I have done in this
> commit) instead of the previous package private access. I checked that no
> other code tries to access this package private `URL#hostAddress`.
>
> I can't think of an easy way to add a jtreg test for this change, so I
> haven't added one.
On Sat, 16 Jan 2021 12:40:47 GMT, Jaikiran Pai wrote:
> As noted by the reporter in the comments of
> https://bugs.openjdk.java.net/browse/JDK-7146776, the `URLStreamHandler` in
> its `getHostAddress` is incorrectly synchronizing on the `URLStreamHandler`
> instance, instead of s
Hello Chris,
I missed this thread previously. This suggested change to the javadoc
looks good to me and the example now makes it clear on what to expect.
As for the last paragraph of the javadoc, I'm just adding to the
suggestions already made by others in this thread. Perhaps the following
On Wed, 3 Feb 2021 11:36:28 GMT, Julia Boes wrote:
>> Maybe could say "rather than parsing the output of toString()" instead of
>> "rather than parsing the string representation". Might be clearer?
>
> Ok, how about this: "rather than parsing the string returned by this
> toString() method"?
T
Can I please get a review for this change which proposes to fix the issue
reported in https://bugs.openjdk.java.net/browse/JDK-8260366?
The issue relates to the concurrent classloading of
`sun.net.ext.ExtendedSocketOptions` and `jdk.net.ExtendedSocketOptions` leading
to a deadlock. This is beca
On Wed, 17 Feb 2021 13:09:27 GMT, Daniel Fuchs wrote:
> Since the test should be fast, I'd also advise to stick several identical
> @run lines (maybe e.g. 5 of them) to increase the probability of the deadlock
> to happen...
Done. The PR has been updated to run this test multiple times now.
-
On Wed, 17 Feb 2021 13:11:23 GMT, Daniel Fuchs wrote:
>> Jaikiran Pai has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Incorporate the review suggestion to run the test multiple times to
>> improve
On Wed, 17 Feb 2021 13:37:19 GMT, Jaikiran Pai wrote:
>> Since the test should be fast, I'd also advise to stick several identical
>> @run lines (maybe e.g. 5 of them) to increase the probability of the
>> deadlock to happen...
>
>> Since the test should b
rst. I'll need input on whether I should
worry about this case or if it's fine in its current form in this PR.
>
> This PR also contains a jtreg test which reproduces the issue and verifies
> the fix.
Jaikiran Pai has updated the pull request incrementally with three additional
commit
On Wed, 17 Feb 2021 16:47:51 GMT, Vyom Mani Tewari
wrote:
> Changes looks OK to me, any specific reason for removing "final" specifier
> from "getInstance" & "register" methods ?.
Hello Vyom, thank you for the review. Since those two methods are `static`, the
`final` was redundant on them and
On Wed, 17 Feb 2021 20:01:17 GMT, Andrey Turbanov
wrote:
>> Jaikiran Pai has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Incorporate the review suggestion to run the test multiple times to
>> improve
rst. I'll need input on whether I should
worry about this case or if it's fine in its current form in this PR.
>
> This PR also contains a jtreg test which reproduces the issue and verifies
> the fix.
Jaikiran Pai has updated the pull request incrementally with one additional
com
On Thu, 18 Feb 2021 04:31:56 GMT, Vyom Mani Tewari
wrote:
> What about improving the test little bit, your test wants to load both
> classes at the same time. Please have a look on modified test.
Hello Vyom, I think that's a good suggestion to use a latch for deciding when
to trigger the clas
On Thu, 18 Feb 2021 07:03:07 GMT, Andrey Turbanov
wrote:
>> Hello @turbanoff, do you mean why read it twice - once here and once inside
>> the `synchronized` block?
>
> Yes. Once here and once inside `synchronized` block.
> Reading `volatile` fields cost _something_ on some architectures, so I
On Thu, 18 Feb 2021 07:59:45 GMT, Andrey Turbanov
wrote:
>> Hello @turbanoff, the double read is necessary to correctly avoid any race
>> conditions and is a typical strategy used in cases like these. I am not
>> aware of any other alternate more performant strategy, for code like this.
>
> Le
rst. I'll need input on whether I should
worry about this case or if it's fine in its current form in this PR.
>
> This PR also contains a jtreg test which reproduces the issue and verifies
> the fix.
Jaikiran Pai has updated the pull request incrementally with one additional
commi
On Thu, 18 Feb 2021 09:57:19 GMT, Jaikiran Pai wrote:
>> Let me be more clear: I think that it's enough to have only 2 `volatile`
>> field reads _in worst case_. We can use local variable to avoid more than
>> needed reads.
>> Current code can perform read 4
1 - 100 of 269 matches
Mail list logo
