Hi Daniel,
you are right, Alan is talking the same , in webrev that i have posted code
is not properly formatted.
In my local repo code is properly formatted.
Thanks,
Vyom
On Wed, Jul 15, 2020 at 2:55 PM Daniel Fuchs
wrote:
> Hi Vyom,
>
> I don't know if that's what Alan is referring to but I
Hi Vyom,
I don't know if that's what Alan is referring to but I see
this:
bsd_close.c:
missing space just after `if` extra space just before `timeout`:
475 if( timeout > 0) {
linux_close.c:
missing space just after `if`:
440 if(timeout > 0) {
missing space just afte
Hi Alan,
thanks for the review, I will definitely fix any formatting issue before
pushing the patch. My local repo code is properly formatted and i was
suspecting that webrev is ignoring the space while generating the patch
file that's why you are seeing the formatting issue.
My local code is p
On 14/07/2020 20:09, Daniel Fuchs wrote:
On 12/07/2020 07:36, Vyom Tiwari wrote:
Hi Patrick,
Thanks for testing, Alan, Daniel can i get the final review comment
from you both ?.
Hi Vyom,
http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/index.html
Vyom - will you fix the formattin
On 12/07/2020 07:36, Vyom Tiwari wrote:
Hi Patrick,
Thanks for testing, Alan, Daniel can i get the final review comment
from you both ?.
Hi Vyom,
http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/index.html
looks good to me.
best regards,
-- daniel
Hi Patrick,
Thanks for testing, Alan, Daniel can i get the final review comment from
you both ?.
Thanks,
Vyom
On Wed, Jul 8, 2020 at 8:17 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:
> Hi,
>
> No problem.
>
> I ran our tests against your latest patch, and everything passed.
>
>
Hi,
No problem.
I ran our tests against your latest patch, and everything passed.
Thanks Vyom.
Kind regards,
Patrick
> On 8 Jul 2020, at 06:49, Vyom Tiwari wrote:
>
> Hi Patrick,
> Thanks for testing, please find the latest
> webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/in
Hi Patrick,
Thanks for testing, please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev1.0/index.html). I fixed
the windows build issue.
Thanks,
Vyom
On Tue, Jul 7, 2020 at 11:49 PM Patrick Concannon <
patrick.concan...@oracle.com> wrote:
> Hi Vyom,
>
> I imported your l
Hi Vyom,
I imported your latest patch and ran it on our test system, and I noticed the
following error on Windows:
[2020-07-07T11:09:20,621Z]
T:\workspace\open\test\jdk\java\net\Socket\libNativeThread.c(54) : error C2220:
the following warning is treated as an error
[2020-07-07T11:09:20,621Z]
Hi All,
Please find the updated webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.9/index.html). I leave
the idea of using the PoolCleaner.
Thanks,
Vyom
On Sat, Jul 4, 2020 at 9:08 PM Martin Buchholz wrote:
> Right. It would be a project to create a jtreg test utility inspired
> by
Right. It would be a project to create a jtreg test utility inspired
by PoolCleaner and use it in many tests.
On Sat, Jul 4, 2020 at 8:24 AM Vyom Tiwari wrote:
>
> Hi Martin
> Thanks for the review, I will try to address your review comment.
>
> I wanted to write a simple test case for this issu
Hi Martin
Thanks for the review, I will try to address your review comment.
I wanted to write a simple test case for this issue but it is getting more
complex.
Thanks,
Vyom
On Sat, Jul 4, 2020 at 8:14 PM Martin Buchholz wrote:
> On Fri, Jul 3, 2020 at 11:12 PM Alan Bateman
> wrote:
>
> > - "s
Hi Alan,
Thanks for the review, I passed ss to sendSignal because in failing cases
ss will throw an exception on the first SIGPIPE signal so i need to put a
check if the server socket is not closed.
Same reason for L64,in failing case server socket will be closed after
SIGPIPE. I will addres
On 04/07/2020 15:44, Martin Buchholz wrote:
On Fri, Jul 3, 2020 at 11:12 PM Alan Bateman wrote:
- "service" isn't a great name for the Executor. Also you can make use
of try-finally, e.g.
ExecutorService executor = Executors.newFixedThreadPool(1);
try { ... } finally { executor.shutdown();
On Fri, Jul 3, 2020 at 11:12 PM Alan Bateman wrote:
> - "service" isn't a great name for the Executor. Also you can make use
> of try-finally, e.g.
> ExecutorService executor = Executors.newFixedThreadPool(1);
> try { ... } finally { executor.shutdown(); }
If you want to do this structured-concu
On 30/06/2020 06:39, Vyom Tiwari wrote:
Thanks for review please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.8/index.html).
I fixed the Windows build issue.
Minor nit but I assume build-dev will want to keep the indentation in
TestFilesCompilation.gmk consist
Hi Daniel,
Thanks for review please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.8/index.html). I fixed
the Windows build issue.
Thanks,
Vyom
On Mon, Jun 29, 2020 at 10:23 PM Daniel Fuchs
wrote:
> Hi Vyom,
>
>
> On 29/06/2020 08:03, Vyom Tiwari wrote:
> > Hi Alan
Hi Vyom,
On 29/06/2020 08:03, Vyom Tiwari wrote:
Hi Alan,Daniel,
Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.7/index.html). I
have added a new native method and removed the hardcoding(SIGPIPE).
That fails to build on windows:
[2020-06-29T15:26:16,899Z
Hi Alan,Daniel,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.7/index.html). I have
added a new native method and removed the hardcoding(SIGPIPE).
I gave a thought on creating a separate thread and sending a signal but it
will further increase the complexity
Hi Alan,
Thanks for the review comment, we can send multiple signals for sure. I
will create a thread which will send multiple signals.
Creating a new native method that returns a signal is a bit too much as
SIGPIPE is a standard signal, but I will add a native method as you
suggested.
I think
On 26/06/2020 05:17, Vyom Tiwari wrote:
Hi Daniel/Alan,
Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.6/index.html).
I think we can do better than one signal after 200ms. Have you given any
thought to have a thread that signals many times so that you have a
Hi Daniel/Alan,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.6/index.html).
Thanks,
Vyom
On Wed, Jun 24, 2020 at 5:32 PM Vyom Tiwari wrote:
> let me refactor the test, i will try to incorporate your suggestion.
> Vyom
>
>
> On Wed, Jun 24, 2020 at 5:21 PM Da
let me refactor the test, i will try to incorporate your suggestion.
Vyom
On Wed, Jun 24, 2020 at 5:21 PM Daniel Fuchs
wrote:
> On 24/06/2020 12:14, Vyom Tiwari wrote:
> > 3-> Looking at SocketAcceptInterruptTest it looks like the test
> > will succeed if it manages to send the signal - and tha
On 24/06/2020 12:14, Vyom Tiwari wrote:
3-> Looking at SocketAcceptInterruptTest it looks like the test
will succeed if it manages to send the signal - and that's it.
Is it really a measure of success?
No, I explained in question 2.
So basically you rely on the test infrastructure having
regi
Hi Daniel,
Thanks for review, please find my answers below.
1-> What is the purpose of calling NativeThread.close?
To close the thread which is waiting in SocketAccept, I wrote a test in
such a way that the server thread will wait infinitely in socket accept and
there will not be any incoming so
Hi Vyom,
What is the purpose of calling NativeThread.close?
Is that just to clean up and make the server thread terminate?
If so there should be better ways to do that.
When does the test succeed?
Looking at SocketAcceptInterruptTest it looks like the test
will succeed if it manages to send the
Hi Alan,
thanks for the review, i think there is some problem with webrev, in my
local repo linux_close.c, bsd_close.c both are properly formatted.
I will do the suggested changes in the tests and will send the updated
webrev soon.
Thanks,
Vyom
On Wed, Jun 24, 2020 at 11:49 AM Alan Bateman
wro
On 24/06/2020 05:53, Vyom Tiwari wrote:
Hi Daniel/Alan,
Thanks for the review , I was not sure how JVM/JDK handles the signals
that's why I installed my own signal handler. As you both pointed out,
I removed the custom signal handler in NativeThread.
Please find the latest
webrev(http://cr.
Hi Daniel/Alan,
Thanks for the review , I was not sure how JVM/JDK handles the signals
that's why I installed my own signal handler. As you both pointed out, I
removed the custom signal handler in NativeThread.
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.5/i
Hi Vyom,
Just to let you know that the new SocketAcceptInterruptTest test
seems to have some stability issues. Patrick imported your test
and ran it on our test system.
We got one crash (SIGSEV) on macOS with the first @run that uses
the old impl, and a timeout with the second @run that uses the
On 18/06/2020 09:02, Vyom Tiwari wrote:
Hi,
Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.4/index.html)
where i fix the build issue as well.
The update to linux_close.c and bsd_close.c look okay but please use "if
(timeout > 0) {" to keep the code consiste
Hi,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.4/index.html) where i
fix the build issue as well.
Thanks,
Vyom
On Wed, Jun 17, 2020 at 3:34 PM Vyom Tiwari wrote:
> Hi Alan,
>
> Please find the latest webrev(
> http://cr.openjdk.java.net/~vtewari/8237858/
Hi Alan,
Please find the latest webrev(
http://cr.openjdk.java.net/~vtewari/8237858/webrev0.3/index.html). I tried
to incorporate the suggestion given by you.I created the NativeThread class
which has functions that send signals to threads.
Note: I am facing some compilation issue when i am tr
On 17/03/2020 05:21, Vyom Tiwari wrote:
Hi Daniel/Michael,
Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.2/index.html),
where i put a test case as well.
Can you look at make/test/JtregNativeJdk.gmk? That should give you ideas
on how to develop tests th
man
> *Gesendet:* Wednesday, March 11, 2020 9:21:24 AM
> *An:* Vyom Tewari26
> *Cc:* net-dev@openjdk.java.net
> *Betreff:* Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR
> incorrectly
>
> On 11/03/2020 08:09, Vyom Tewari26 wrote:
>
> Hi Alan,
>
>
ftrag von Alan Bateman
Gesendet: Wednesday, March 11, 2020 9:21:24 AM
An: Vyom Tewari26
Cc: net-dev@openjdk.java.net
Betreff: Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR
incorrectly
On 11/03/2020 08:09, Vyom Tewari26 wrote:
Hi Alan,
Thanks for comment yes for >=JDK13 we a
On 11/03/2020 08:09, Vyom Tewari26 wrote:
Hi Alan,
Thanks for comment yes for >=JDK13 we are using new socket impl which
does not have this problem, i am not planning to wright a test, to
test this issue we need to get the native thread ID then we have to
interrupt the thread to see if JDK c
Hi Alan,
Thanks for comment yes for >=JDK13 we are using new socket impl which does not have this problem, i am not planning to wright a test, to test this issue we need to get the native thread ID then we have to interrupt the thread to see if JDK code is behaving as expected.
I tested this
Vyom,
Are you planning to create a native test for this? I realize the fix you
are discussing here isn't too interesting for >= JDK 13 but I think it
could be useful to ensure that we have a test that signals threads in
all of the blocking operations (connect might be tricky butt at least
rea
dable.>> I hosted the patch to>> cr.openjdk.net(http://cr.openjdk.java.net/~vtewari/8237858/webrev/index.html ).>> Thanks,>> Vyom>>>> - Original message ->> From: Daniel Fuchs >> To: Vyom Tewari26 , net-dev@openjdk.java.net>>
to make code> more readable.> I hosted the patch to> cr.openjdk.net(http://cr.openjdk.java.net/~vtewari/8237858/webrev/index.html ).> Thanks,> Vyom>> - Original message -> From: Daniel Fuchs > To: Vyom Tewari26 , net-dev@openjdk.java.net> Cc:> Su
va.net/~vtewari/8237858/webrev/index.html).
Thanks,
Vyom
- Original message -
From: Daniel Fuchs
To: Vyom Tewari26 , net-dev@openjdk.java.net
Cc:
Subject: [EXTERNAL] Re: RFR 8237858: PlainSocketImpl.socketAccept()
handles EINTR incorrectly
Date: Tue, Mar 10, 2
osted the patch to
cr.openjdk.net(http://cr.openjdk.java.net/~vtewari/8237858/webrev/index.html).
Thanks,
Vyom
- Original message -
From: Daniel Fuchs
To: Vyom Tewari26 , net-dev@openjdk.java.net
Cc:
Subject: [EXTERNAL] Re: RFR 8237858: PlainSocketImpl.socketAccept()
Hi Daniel,
Thanks for review and running all tests, In past we(i am the culprit) removed the "if(timeout > 0) {" condition by mistake. This check is to make sure that if the timeout set only then we do all other time calculation but somehow it was not very clear by reading the code.
NET_Timeou
The change looks okay to me. Though the comment about the -1 case also
applies
if timeout is 0. The behavior is still okay in that case, but the
comment should acknowledge that,
however unlikely it is to occur.
- Michael.
On 10/03/2020 16:37, Daniel Fuchs wrote:
Hi Vyom,
I have sent your pro
Hi Vyom,
I have sent your proposed fix to our test system and observed no
regression. I agree your proposed changes seem to address the
issue adequately. However, I'd like to hear a second opinion on the
possible side effects of this fix, since NET_Timeout may be called
at many other places.
I s
Ping,
please review the below code.
Vyom
- Original message -From: "Vyom Tewari26" Sent by: "net-dev" To: net-dev@openjdk.java.netCc:Subject: [EXTERNAL] RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectlyDate: Tue, Feb 25, 2020 10:07 PM
Hi ,
Please find the below
Hi ,
Please find the below fix for the issue(https://bugs.openjdk.java.net/browse/JDK-8237858). In "PlainSocketImpl_socketAccept" did not handle -1 timeout properly.
In case of -1 timeout, "PlainSocketImpl_socketAccept" calls "NET_Timeout" if it is interrupted by signal(EINTR) then in case of
48 matches
Mail list logo
