Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly
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 read, write, and accept). -Alan
RE: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly
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 issue at my local Linux env, what i did is modify the java.lang.Thread.getId() to always return the native thread id of main thread and use this native thread id in the test program to send a signal(kill -SIGPIPE threadid) and i checked that JDK code is behaving as expected. We can try to write a test case(which requires some native code as well) which simulate the above but that will be lot of work. We can file a new issue to write a test which test how(read,write,accept) operations behave when they are interrupted by a signal. Alan, what do you think ?. Thanks, Vyom - Original message -From: Alan Bateman To: Vyom Tewari26 Cc: net-dev@openjdk.java.netSubject: [EXTERNAL] Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectlyDate: Wed, Mar 11, 2020 12:49 PM Vyom,Are you planning to create a native test for this? I realize the fix youare discussing here isn't too interesting for >= JDK 13 but I think itcould be useful to ensure that we have a test that signals threads inall of the blocking operations (connect might be tricky butt at leastread, write, and accept).-Alan
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 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 issue at my local Linux env, what i did is modify the java.lang.Thread.getId() to always return the *native thread id of main thread* and use this native thread id in the test program to send a signal(kill -SIGPIPE threadid) and i checked that JDK code is behaving as expected. We can try to write a test case(which requires some native code as well) which simulate the above but that will be lot of work. We can file a new issue to write a test which test how(read,write,accept) operations behave when they are interrupted by a signal. Alan, what do you think ?. Native tests used to be hard but there is supporting infrastructure in the build for some time that makes it a lot easier. In this case, I think it should be possible to invoke a JNI method that returns the native thread id and then it can be signalled when the thread is blocked in accept (or read or write). So it shouldn't be too hard, it comes to whether anyone has time. -Alan.
Re: RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly
Hello, Depending on the test environment you don't need much native tools. You can use java.io.File to open /proc/thread-self to get the TID and use ProcessBuilder to execute the kill command. Gruss Bernd -- http://bernd.eckenfels.net Von: net-dev im Auftrag 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 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 issue at my local Linux env, what i did is modify the java.lang.Thread.getId() to always return the native thread id of main thread and use this native thread id in the test program to send a signal(kill -SIGPIPE threadid) and i checked that JDK code is behaving as expected. We can try to write a test case(which requires some native code as well) which simulate the above but that will be lot of work. We can file a new issue to write a test which test how(read,write,accept) operations behave when they are interrupted by a signal. Alan, what do you think ?. Native tests used to be hard but there is supporting infrastructure in the build for some time that makes it a lot easier. In this case, I think it should be possible to invoke a JNI method that returns the native thread id and then it can be signalled when the thread is blocked in accept (or read or write). So it shouldn't be too hard, it comes to whether anyone has time. -Alan.
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
Hi Alan, On 10/03/2020 19:59, Alan Bateman wrote: On 10/03/2020 18:32, Patrick Concannon wrote: http://cr.openjdk.java.net/~pconcannon/8239355/webrevs/webrev.02 Thanks for adding a test for getOption(SO_SNDBUF). That test (testGetOption) should be checking that SO_SNDBUF is >= expected value as it's okay for net.inet.udp.maxdgram to have a larger than what the test expects. +1 testSend sends to the loopback address but I think we need this test to send datagrams on the network (sending to the loopback is okay too but I think you want this test to send a datagram on the network because we want fragmentation on the network(. Do we really? I am not sure we do. We just want to verify that we don't get the "packet too large" exception caused by the SO_SNDBUF buffer being too small. In other words, we want to check that setting SO_SNDBUF was effective and that it was really passed to the underlying system stack and taken into account. But maybe you have a different scenario in mind? Using the loopback is also expedient because a machine that has IPv6 might not have an IPv6 external address configured, but it should have an IPv6 loopback (::1) always. I assume we could loop over the network interfaces and try to find one that has an IPv6 address configured which is not the loopback - but that complicates the test. We can do it if there's a strong reason to do it (we don't want to test that the network itself actually supports ~64k datagrams, we just want to test that we would be able to send them if it supported it?) The java.net.preferIPv6Addresses system property is about configuring the order of name service lookup. These runs shouldn't impact anything here, dual and preferIPv4Stack=true should be all that is needed. Yes and no - and the test is there to verify that it doesn't have any unexpected side effects (we know it shouldn't). A minor nit is that we should probably find a name for the test that is consistent with the other tests in this area. Something like LargeDatagram or MinSendBufferSize is okay. +1 best regards, -- daniel -Alan
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
On 11/03/2020 10:58, Daniel Fuchs wrote: : testSend sends to the loopback address but I think we need this test to send datagrams on the network (sending to the loopback is okay too but I think you want this test to send a datagram on the network because we want fragmentation on the network(. Do we really? I am not sure we do. We just want to verify that we don't get the "packet too large" exception caused by the SO_SNDBUF buffer being too small. It would be great if we had a test to send large datagrams on the network as that is the only way to properly test that they can be re-assembled and received. I don't mind if it's a separate test, I agree it can be tricky on systems that have unusual configurations. I'm just pointing out that testing that the send doesn't fail and that the datagram can be received through the loopback may not be enough here. : The java.net.preferIPv6Addresses system property is about configuring the order of name service lookup. These runs shouldn't impact anything here, dual and preferIPv4Stack=true should be all that is needed. Yes and no - and the test is there to verify that it doesn't have any unexpected side effects (we know it shouldn't). My preference would be to drop these so the test only runs twice. The reason is that the effect of setting java.net.preferIPv6Addresses is not widely known and it will confuse anyone that needs to maintain this test in the future. -Alan
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
Hi Alan, On 11/03/2020 12:08, Alan Bateman wrote: Do we really? I am not sure we do. We just want to verify that we don't get the "packet too large" exception caused by the SO_SNDBUF buffer being too small. It would be great if we had a test to send large datagrams on the network as that is the only way to properly test that they can be re-assembled and received. I don't mind if it's a separate test, I agree it can be tricky on systems that have unusual configurations. I'm just pointing out that testing that the send doesn't fail and that the datagram can be received through the loopback may not be enough here. Our test doesn't even try to receive the datagram. It just sends it. We can write a new test that does the full roundtrip if you think it's worthwile. Yes and no - and the test is there to verify that it doesn't have any unexpected side effects (we know it shouldn't). My preference would be to drop these so the test only runs twice. The reason is that the effect of setting java.net.preferIPv6Addresses is not widely known and it will confuse anyone that needs to maintain this test in the future. OK - then we can remove the preferIPv6Addresses property and replace: 114 populateDataProvider(testcases, datagramChannel, MAX_PAYLOAD, 117 null); with if (hasIPv4()) populateDataProvider(testcases, datagramChannel, IPV4_SNDBUF, StandardProtocolFamily.INET); if (hasIPv6() && !preferIPv4Stack()) populateDataProvider(testcases, datagramChannel, IPV6_SNDBUF, StandardProtocolFamily.INET6); to verify that the channel opened with the no-arg DatagramChannel.open() can be used to send both IPv4 and IPv6 datagrams. cheers, -- daniel
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
Hi, I've included those additional changes, and they can be found in the new webrev below. http://cr.openjdk.java.net/~pconcannon/8239355/webrevs/webrev.03/ Kind regards, Patrick On 11/03/2020 12:27, Daniel Fuchs wrote: Hi Alan, On 11/03/2020 12:08, Alan Bateman wrote: Do we really? I am not sure we do. We just want to verify that we don't get the "packet too large" exception caused by the SO_SNDBUF buffer being too small. It would be great if we had a test to send large datagrams on the network as that is the only way to properly test that they can be re-assembled and received. I don't mind if it's a separate test, I agree it can be tricky on systems that have unusual configurations. I'm just pointing out that testing that the send doesn't fail and that the datagram can be received through the loopback may not be enough here. Our test doesn't even try to receive the datagram. It just sends it. We can write a new test that does the full roundtrip if you think it's worthwile. Yes and no - and the test is there to verify that it doesn't have any unexpected side effects (we know it shouldn't). My preference would be to drop these so the test only runs twice. The reason is that the effect of setting java.net.preferIPv6Addresses is not widely known and it will confuse anyone that needs to maintain this test in the future. OK - then we can remove the preferIPv6Addresses property and replace: 114 populateDataProvider(testcases, datagramChannel, MAX_PAYLOAD, 117 null); with if (hasIPv4()) populateDataProvider(testcases, datagramChannel, IPV4_SNDBUF, StandardProtocolFamily.INET); if (hasIPv6() && !preferIPv4Stack()) populateDataProvider(testcases, datagramChannel, IPV6_SNDBUF, StandardProtocolFamily.INET6); to verify that the channel opened with the no-arg DatagramChannel.open() can be used to send both IPv4 and IPv6 datagrams. cheers, -- daniel
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
Regarding the test to check that large datagrams are sent and received correctly across a network: I've created a new issue for it and included the link below. https://bugs.openjdk.java.net/browse/JDK-8240901 Kind regards, Patrick On 11/03/2020 16:54, Patrick Concannon wrote: Hi, I've included those additional changes, and they can be found in the new webrev below. http://cr.openjdk.java.net/~pconcannon/8239355/webrevs/webrev.03/ Kind regards, Patrick On 11/03/2020 12:27, Daniel Fuchs wrote: Hi Alan, On 11/03/2020 12:08, Alan Bateman wrote: Do we really? I am not sure we do. We just want to verify that we don't get the "packet too large" exception caused by the SO_SNDBUF buffer being too small. It would be great if we had a test to send large datagrams on the network as that is the only way to properly test that they can be re-assembled and received. I don't mind if it's a separate test, I agree it can be tricky on systems that have unusual configurations. I'm just pointing out that testing that the send doesn't fail and that the datagram can be received through the loopback may not be enough here. Our test doesn't even try to receive the datagram. It just sends it. We can write a new test that does the full roundtrip if you think it's worthwile. Yes and no - and the test is there to verify that it doesn't have any unexpected side effects (we know it shouldn't). My preference would be to drop these so the test only runs twice. The reason is that the effect of setting java.net.preferIPv6Addresses is not widely known and it will confuse anyone that needs to maintain this test in the future. OK - then we can remove the preferIPv6Addresses property and replace: 114 populateDataProvider(testcases, datagramChannel, MAX_PAYLOAD, 117 null); with if (hasIPv4()) populateDataProvider(testcases, datagramChannel, IPV4_SNDBUF, StandardProtocolFamily.INET); if (hasIPv6() && !preferIPv4Stack()) populateDataProvider(testcases, datagramChannel, IPV6_SNDBUF, StandardProtocolFamily.INET6); to verify that the channel opened with the no-arg DatagramChannel.open() can be used to send both IPv4 and IPv6 datagrams. cheers, -- daniel
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
On 11/03/2020 16:54, Patrick Concannon wrote: Hi, I've included those additional changes, and they can be found in the new webrev below. http://cr.openjdk.java.net/~pconcannon/8239355/webrevs/webrev.03/ Thanks for the update, looks good. I assume the updates to the IPSupport infra isn't needed but it might be needed some day so okay to include. I see your mail with the link to JDK-8240901 to add tests that send/receive large datagrams so I think we're good. -Alan.
