[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8220175: serviceability/dcmd/framework/VMVersionTest.java fails with a timeout
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2019-06-20 19:13:12
Message-ID: 5ddcfc80-96db-202a-2d23-823441705156 () oracle ! com
[Download RAW message or body]

Hi Daniil,

It looks good to me.

Thanks,
Serguei


On 6/19/19 21:02, Daniil Titov wrote:
> Please review the change that fixes an intermittent failure of \
> serviceability/dcmd/framework/* tests on Linux platform. 
> The problem here is that get_namespace_pid() method, that is called by \
> mmap_attach_shared () that in turn is called by PerfMemory::attach(), tries to read \
> the namespace pid information from /proc/<pid>/status file. However, it doesn't \
> check that the error indicator associated with stream is set that results in the \
> endless  loop (lines 664-677) if the process terminates after /proc/<pid>/status \
> was opened (line 659) and checked for null (line 661).
> 
> 658	  snprintf(fname, sizeof(fname), "/proc/%d/status", vmid);
> 659	  FILE *fp = fopen(fname, "r");
> 660	
> 661	  if (fp) {
> 662	    int pid, nspid;
> 663	    int ret;
> 664	    while (!feof(fp)) {
> 665	      ret = fscanf(fp, "NSpid: %d %d", &pid, &nspid);
> 666	      if (ret == 1) {
> 667	        break;
> 668	      }
> 669	      if (ret == 2) {
> 670	        retpid = nspid;
> 671	        break;
> 672	      }
> 673	      for (;;) {
> 674	        int ch = fgetc(fp);
> 675	        if (ch == EOF || ch == (int)'\n') break;
> 676	      }
> 677	    }
> 678	    fclose(fp);
> 679	  }
> 
> The fix adds the check for the error indicator to ensure that the "while" loop \
> terminates properly if the file no longer exists. 
> Issues [3] and [4] have the same cause and will be closed as duplicates of this \
> issue. 
> Testing: Mach5 hotspot_serviceability tests succeeded, tier1,tier2, and tier3 tests \
> are in progress. 
> [1] Webrev: http://cr.openjdk.java.net/~dtitov/8220175/webrev.01/
> [2] Bug: https://bugs.openjdk.java.net/browse/JDK-8220175
> [3] https://bugs.openjdk.java.net/browse/JDK-8223600
> [4] https://bugs.openjdk.java.net/browse/JDK-8217351
> 
> Thanks!
> -Daniil
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]