RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails intermittently on Windows and Solaris
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Wed Feb 21 06:16:53 UTC 2018
Hi David,
On 2/20/18 20:02, David Holmes wrote:
> Hi Daniil,
>
> Good find on this!
>
> What does the actual spec say about the length of things and how they
> may be split across multiple packets? Are we guaranteed that at most
> two packets will be involved? What about for other things eg:
>
> 68 protected byte[] readJdwpString(DataInputStream ds) throws
> IOException {
> 69 byte[] str = null;
> 70 int len = ds.readInt();
> 71 if (len > 0) {
> 72 str = new byte[len];
> 73 ds.read(str, 0, len);
> 74 }
>
> might we get a short-read of the string if it is split across multiple
> packets?
Nice catch!
Even though this fix is enough to resolve this problem now, there is a
chance,
it can fail in the future when more modules are added to the platform.
> I'm wondering if all these reads should be loops, ensuring we read the
> expected amount of data.
>
> One further comment - not sure why we need the print out for when we
> do read multiple packets?
> That would seem to be a debugging aid.
Yes, it helps to understand what happens.
Many tests have a lack of tracing which makes it harder to debug and
understand failures.
Thanks,
Serguei
>
> Thanks,
> David
>
> On 21/02/2018 10:14 AM, Daniil Titov wrote:
>> Hi Serguei,
>>
>> A new version of the webrev that has these strings reformatted is at
>> http://cr.openjdk.java.net/~dtitov/8170541/webrev.02/
>>
>> Thank you!
>>
>> Best regards,
>>
>> Daniil
>>
>> *From: *"serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
>> *Date: *Tuesday, February 20, 2018 at 3:00 PM
>> *To: *Daniil Titov <daniil.x.titov at oracle.com>,
>> "serviceability-dev at openjdk.java.net"
>> <serviceability-dev at openjdk.java.net>
>> *Subject: *Re: RFR 8170541:
>> serviceability/jdwp/AllModulesCommandTest.java fails intermittently
>> on Windows and Solaris
>>
>> Hi Daniil,
>>
>> Interesting issue...
>> Thank you for finding to the root cause so quickly!
>>
>> The fix looks good.
>> Could I ask you to reformat these lines to make the L54 shorter ?:
>>
>> 54 System.out.println("[" + getClass().getName() +
>> "] Only " + bytesRead + " bytes of " + dataLength +
>>
>> 55 " were read in the first packet.
>> Reading the rest...");
>>
>> Thanks,
>> Serguei
>>
>>
>> On 2/20/18 09:24, Daniil Titov wrote:
>>
>> Please review the changes that fix intermittent failure of
>> serviceability/jdwp/AllModulesCommandTest.java test.
>>
>> The problem here is that for a large data the JDWP agent
>> (socketTransport_writePacket() method in
>> src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c )
>> sends 2 packets and in some cases only the first packet is received
>> at the time when the test reads the reply from the JDWP agent. Since
>> the test does not check that all data is received in the first
>> packet the correlation between commands and replies became broken
>> (the unread second packet is read by the next command and the reply
>> for the next command is read by the next after next command and
>> so on).
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170541
>>
>> Webrev: http://cr.openjdk.java.net/~dtitov/8170541/webrev.01
>>
>> The tests ran successfully with Mach5.
>>
>> Best regards,
>>
>> Daniil
>>
>>
>>
More information about the serviceability-dev
mailing list