Hi Daniil,
I think your fix is good, although I wonder about the robustness of this
function given that the status file can change while it is reading it. I
wonder if it can return a false negative because it never saw the
matching line. This could happen if a line gets deleted, causing the
matching line to suddenly be earlier in the file, possibly before the
current location being read. Anyway, that's not really related to the
current issue or fix, but if you think it might be an issue maybe a bug
should be filed for it.
thanks,
Chris
On 6/19/19 9:02 PM, 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
