On Fri, 19 Feb 2021 04:26:26 GMT, Jaikiran Pai wrote:
> After thinking a bit more about this issue and the patch I had proposed, I
> realized what I did wrong in this patch. The `synchronized` block in the
> `sun.net.ext.ExtendedSocketOptions#getInstance()` was scoped one statement
> too many.
On Thu, 18 Feb 2021 13:06:44 GMT, Jaikiran Pai wrote:
>> Changes requested by dfuchs (Reviewer).
>
> I'm closing this for now, for the reason stated here
> https://github.com/openjdk/jdk/pull/2601#discussion_r578400669.
After thinking a bit more about this issue and the patch I had proposed, I
> 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.
On Thu, 18 Feb 2021 21:43:00 GMT, Alex Menkov wrote:
> The fix also partially fixes JdwpAttachTest failures (JDK-8253940).
> The failures are caused by network configuration changes during test
> execution ("global" IPv6 addresses disappears from interface).
> To minimize chances of intermittent
The fix also partially fixes JdwpAttachTest failures (JDK-8253940).
The failures are caused by network configuration changes during test execution
("global" IPv6 addresses disappears from interface).
To minimize chances of intermittent failures like this java.net tests use only
link-local address
On Mon, 15 Feb 2021 19:47:00 GMT, Andrey Turbanov
wrote:
>> 8080272 Refactor I/O stream copying to use
>> InputStream.transferTo/readAllBytes and Files.copy
>
> Andrey Turbanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8080272: Ref
On Thu, 18 Feb 2021 13:31:07 GMT, Jaikiran Pai wrote:
>> Hi Jaikiran,
>>
>> I tested with my suggested change and i did not see any deadlock at my local
>> Linux environment. I just ran test in loop and it worked as expected.
>>
>> Thanks,
>> Vyom
>
> Hello Vyom,
>
> The trick is to change t
On Thu, 18 Feb 2021 13:23:37 GMT, Vyom Mani Tewari
wrote:
>> Hello Daniel, I had thought about it in my previous commit. But this won't
>> work, since in the normal case where the `ClassNotFoundException` doesn't
>> get thrown, the `instance` is actually set in the `register` method which
>>
On Thu, 18 Feb 2021 13:05:48 GMT, Jaikiran Pai wrote:
>> src/java.base/share/classes/sun/net/ext/ExtendedSocketOptions.java line 189:
>>
>>> 187: }
>>> 188: }
>>> 189: return instance;
>>
>> I'd suggest changing these two lines as well:
>>
>> // the
On Thu, 18 Feb 2021 10:54:36 GMT, Daniel Fuchs wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> reduce volatile reads
>
> Changes requested by dfuchs (Reviewer).
I'm closing this for now, for the reason stated here
On Wed, 17 Feb 2021 07:20:27 GMT, Jaikiran Pai wrote:
> 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 `j
On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов
wrote:
>> Non-static classes hold a link to their parent classes, which in many cases
>> can be avoided.
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8261880: Remove s
On Thu, 18 Feb 2021 10:44:03 GMT, Jaikiran Pai wrote:
>> 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` a
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 times _in worst case_: twice outside
> 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.
On Thu, 18 Feb 2021 07:12:34 GMT, Andrey Turbanov
wrote:
>> Hi @turbanoff, I'm happy to sponsor but I see two comments by @marschall -
>> have they been addressed?
>> https://github.com/openjdk/jdk/pull/1853#discussion_r572815422
>> https://github.com/openjdk/jdk/pull/1853#discussion_r572380746
On Tue, 16 Feb 2021 13:37:20 GMT, Jamie Le Tual
wrote:
> > I think that the changes are mostly good. I would like to try them out on
> > my local system and our internal buildAndTest system.
>
> Chris, were you able to test the patch?
>
> Also if anyone has an idea as to the best way to unit
On Tue, 16 Feb 2021 13:31:13 GMT, Jamie Le Tual
wrote:
>> src/java.base/unix/native/libnet/Inet6AddressImpl.c line 713:
>>
>>> 711: This usually requires "root" privileges, so it's likely to fail.
>>> 712: If all else fails, fall back to TCP and implement tcp echo
>>> 713: */
>>
>> Thi
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
On Thu, 18 Feb 2021 05:01:41 GMT, Jaikiran Pai 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 redund
On Thu, 18 Feb 2021 07:35:51 GMT, Jaikiran Pai wrote:
>> Yes. Once here and once inside `synchronized` block.
>> Reading `volatile` fields cost _something_ on some architectures, so I think
>> we could optimize it a bit.
>
> Hello @turbanoff, the double read is necessary to correctly avoid any r
21 matches
Mail list logo
