* posix/tst-spawn5 failes under make -j @ 2021-07-09 9:45 Florian Weimer 2021-07-09 13:10 ` Adhemerval Zanella 0 siblings, 1 reply; 5+ messages in thread From: Florian Weimer @ 2021-07-09 9:45 UTC (permalink / raw) To: libc-alpha This patch adds some additional diagonstics: diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c index 277b848794..5ba7c7fadb 100644 --- a/posix/tst-spawn5.c +++ b/posix/tst-spawn5.c @@ -116,7 +116,11 @@ handle_restart (int argc, char *argv[]) fds[i].found = found = true; if (!found) - FAIL_EXIT1 ("unexpected open file descriptor: %ld", fd); + { + char *path = xasprintf ("/proc/self/fd/%s", e->d_name); + char *resolved = xreadlink (path); + FAIL_EXIT1 ("unexpected open file descriptor %ld: %s", fd, resolved); + } } closedir (dirp); It's the pipe from the make job server: error: tst-spawn5.c:122: unexpected open file descriptor 3: pipe:[9958839] I think the test needs to be changed so that lowfd is passed across the restart, and checking only considers descriptors >= lowfd. Comments? Thanks, Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: posix/tst-spawn5 failes under make -j 2021-07-09 9:45 posix/tst-spawn5 failes under make -j Florian Weimer @ 2021-07-09 13:10 ` Adhemerval Zanella 2021-07-09 13:41 ` Florian Weimer 0 siblings, 1 reply; 5+ messages in thread From: Adhemerval Zanella @ 2021-07-09 13:10 UTC (permalink / raw) To: Florian Weimer, libc-alpha On 09/07/2021 06:45, Florian Weimer wrote: > This patch adds some additional diagonstics: > > diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c > index 277b848794..5ba7c7fadb 100644 > --- a/posix/tst-spawn5.c > +++ b/posix/tst-spawn5.c > @@ -116,7 +116,11 @@ handle_restart (int argc, char *argv[]) > fds[i].found = found = true; > > if (!found) > - FAIL_EXIT1 ("unexpected open file descriptor: %ld", fd); > + { > + char *path = xasprintf ("/proc/self/fd/%s", e->d_name); > + char *resolved = xreadlink (path); > + FAIL_EXIT1 ("unexpected open file descriptor %ld: %s", fd, resolved); > + } > } > closedir (dirp); > > It's the pipe from the make job server: > > error: tst-spawn5.c:122: unexpected open file descriptor 3: pipe:[9958839] > > I think the test needs to be changed so that lowfd is passed across the > restart, and checking only considers descriptors >= lowfd. The restart process already receive a list of the expected opened files, for instance on my environment it will re-executed with: handle_restart: 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 handle_restart: 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 handle_restart: 3 handle_restart: handle_restart: 3 4 I am trying to reproduce the issue without much success, even with make -j or make posix/tests -j. Maybe we can limit the search to the argument range: diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c index e04a6a8088..a9a3aa5d8d 100644 --- a/posix/tst-spawn5.c +++ b/posix/tst-spawn5.c @@ -68,6 +68,8 @@ handle_restart (int argc, char *argv[]) int fd; _Bool found; } *fds = xmalloc (sizeof (struct fd_t) * nfds); + int maxfd = 0; + int minfd = INT_MAX; for (int i = 0; i < nfds; i++) { char *endptr; @@ -77,6 +79,11 @@ handle_restart (int argc, char *argv[]) fds[i].fd = fd; fds[i].found = false; + + if (fd < minfd) + minfd = fd; + if (fd > maxfd) + maxfd = fd; } DIR *dirp = opendir (FD_TO_FILENAME_PREFIX); @@ -109,6 +116,8 @@ handle_restart (int argc, char *argv[]) || fd == STDOUT_FILENO || fd == STDERR_FILENO) continue; + if (fd < minfd || fd > maxfd) + continue; bool found = false; for (int i = 0; i < nfds; i++) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: posix/tst-spawn5 failes under make -j 2021-07-09 13:10 ` Adhemerval Zanella @ 2021-07-09 13:41 ` Florian Weimer 2021-07-09 14:17 ` Adhemerval Zanella 0 siblings, 1 reply; 5+ messages in thread From: Florian Weimer @ 2021-07-09 13:41 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha * Adhemerval Zanella: > On 09/07/2021 06:45, Florian Weimer wrote: >> This patch adds some additional diagonstics: >> >> diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c >> index 277b848794..5ba7c7fadb 100644 >> --- a/posix/tst-spawn5.c >> +++ b/posix/tst-spawn5.c >> @@ -116,7 +116,11 @@ handle_restart (int argc, char *argv[]) >> fds[i].found = found = true; >> >> if (!found) >> - FAIL_EXIT1 ("unexpected open file descriptor: %ld", fd); >> + { >> + char *path = xasprintf ("/proc/self/fd/%s", e->d_name); >> + char *resolved = xreadlink (path); >> + FAIL_EXIT1 ("unexpected open file descriptor %ld: %s", fd, resolved); >> + } >> } >> closedir (dirp); >> >> It's the pipe from the make job server: >> >> error: tst-spawn5.c:122: unexpected open file descriptor 3: pipe:[9958839] >> >> I think the test needs to be changed so that lowfd is passed across the >> restart, and checking only considers descriptors >= lowfd. > > The restart process already receive a list of the expected opened files, > for instance on my environment it will re-executed with: > > handle_restart: 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 > handle_restart: 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 > handle_restart: 3 > handle_restart: > handle_restart: 3 4 > > I am trying to reproduce the issue without much success, even with > make -j or make posix/tests -j. Maybe we can limit the search to the > argument range: make -j2 should do it (make -j without argument probably doesn't involve the job server), sorry. The issue is that the verification should start only at lowfd. This will supercede the STDIN_* checks here: if (fd == dirfd (dirp) || fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO) continue; Thanks, Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: posix/tst-spawn5 failes under make -j 2021-07-09 13:41 ` Florian Weimer @ 2021-07-09 14:17 ` Adhemerval Zanella 2021-07-09 18:40 ` Florian Weimer 0 siblings, 1 reply; 5+ messages in thread From: Adhemerval Zanella @ 2021-07-09 14:17 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha On 09/07/2021 10:41, Florian Weimer wrote: > * Adhemerval Zanella: > >> On 09/07/2021 06:45, Florian Weimer wrote: >>> This patch adds some additional diagonstics: >>> >>> diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c >>> index 277b848794..5ba7c7fadb 100644 >>> --- a/posix/tst-spawn5.c >>> +++ b/posix/tst-spawn5.c >>> @@ -116,7 +116,11 @@ handle_restart (int argc, char *argv[]) >>> fds[i].found = found = true; >>> >>> if (!found) >>> - FAIL_EXIT1 ("unexpected open file descriptor: %ld", fd); >>> + { >>> + char *path = xasprintf ("/proc/self/fd/%s", e->d_name); >>> + char *resolved = xreadlink (path); >>> + FAIL_EXIT1 ("unexpected open file descriptor %ld: %s", fd, resolved); >>> + } >>> } >>> closedir (dirp); >>> >>> It's the pipe from the make job server: >>> >>> error: tst-spawn5.c:122: unexpected open file descriptor 3: pipe:[9958839] >>> >>> I think the test needs to be changed so that lowfd is passed across the >>> restart, and checking only considers descriptors >= lowfd. >> >> The restart process already receive a list of the expected opened files, >> for instance on my environment it will re-executed with: >> >> handle_restart: 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 >> handle_restart: 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 >> handle_restart: 3 >> handle_restart: >> handle_restart: 3 4 >> >> I am trying to reproduce the issue without much success, even with >> make -j or make posix/tests -j. Maybe we can limit the search to the >> argument range: > > make -j2 should do it (make -j without argument probably doesn't involve > the job server), sorry. > > The issue is that the verification should start only at lowfd. > This will supercede the STDIN_* checks here: > > if (fd == dirfd (dirp) > || fd == STDIN_FILENO > || fd == STDOUT_FILENO > || fd == STDERR_FILENO) > continue; I usually ran the check with the current processor number in my system in fact (in this case 24) and I haven't see any issue so far. But I think the following patch should fine, can you check if it improves the error you are seeing: diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c index 277b848794..d586662a8e 100644 --- a/posix/tst-spawn5.c +++ b/posix/tst-spawn5.c @@ -68,6 +68,8 @@ handle_restart (int argc, char *argv[]) int fd; _Bool found; } *fds = xmalloc (sizeof (struct fd_t) * nfds); + int maxfd = 0; + int minfd = INT_MAX; for (int i = 0; i < nfds; i++) { char *endptr; @@ -77,6 +79,11 @@ handle_restart (int argc, char *argv[]) fds[i].fd = fd; fds[i].found = false; + + if (fd < minfd) + minfd = fd; + if (fd > maxfd) + maxfd = fd; } DIR *dirp = opendir (FD_TO_FILENAME_PREFIX); @@ -103,11 +110,8 @@ handle_restart (int argc, char *argv[]) FAIL_EXIT1 ("readdir: invalid file descriptor name: /proc/self/fd/%s", e->d_name); - /* Skip the descriptor which is used to enumerate the descriptors. */ - if (fd == dirfd (dirp) - || fd == STDIN_FILENO - || fd == STDOUT_FILENO - || fd == STDERR_FILENO) + /* Skip the descriptor which are not enumerated in arguments. */ + if (fd < minfd || fd > maxfd) continue; bool found = false; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: posix/tst-spawn5 failes under make -j 2021-07-09 14:17 ` Adhemerval Zanella @ 2021-07-09 18:40 ` Florian Weimer 0 siblings, 0 replies; 5+ messages in thread From: Florian Weimer @ 2021-07-09 18:40 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha * Adhemerval Zanella: > On 09/07/2021 10:41, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> On 09/07/2021 06:45, Florian Weimer wrote: >>>> This patch adds some additional diagonstics: >>>> >>>> diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c >>>> index 277b848794..5ba7c7fadb 100644 >>>> --- a/posix/tst-spawn5.c >>>> +++ b/posix/tst-spawn5.c >>>> @@ -116,7 +116,11 @@ handle_restart (int argc, char *argv[]) >>>> fds[i].found = found = true; >>>> >>>> if (!found) >>>> - FAIL_EXIT1 ("unexpected open file descriptor: %ld", fd); >>>> + { >>>> + char *path = xasprintf ("/proc/self/fd/%s", e->d_name); >>>> + char *resolved = xreadlink (path); >>>> + FAIL_EXIT1 ("unexpected open file descriptor %ld: %s", fd, resolved); >>>> + } >>>> } >>>> closedir (dirp); >>>> >>>> It's the pipe from the make job server: >>>> >>>> error: tst-spawn5.c:122: unexpected open file descriptor 3: pipe:[9958839] >>>> >>>> I think the test needs to be changed so that lowfd is passed across the >>>> restart, and checking only considers descriptors >= lowfd. >>> >>> The restart process already receive a list of the expected opened files, >>> for instance on my environment it will re-executed with: >>> >>> handle_restart: 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 >>> handle_restart: 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 >>> handle_restart: 3 >>> handle_restart: >>> handle_restart: 3 4 >>> >>> I am trying to reproduce the issue without much success, even with >>> make -j or make posix/tests -j. Maybe we can limit the search to the >>> argument range: >> >> make -j2 should do it (make -j without argument probably doesn't involve >> the job server), sorry. >> >> The issue is that the verification should start only at lowfd. >> This will supercede the STDIN_* checks here: >> >> if (fd == dirfd (dirp) >> || fd == STDIN_FILENO >> || fd == STDOUT_FILENO >> || fd == STDERR_FILENO) >> continue; > > I usually ran the check with the current processor number in my system > in fact (in this case 24) and I haven't see any issue so far. But I > think the following patch should fine, can you check if it improves the > error you are seeing: The issue is that the test range (lowfd) starts at 4 in my case, while the verification logic assumes it's effectively 3. > > diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c > index 277b848794..d586662a8e 100644 > --- a/posix/tst-spawn5.c > +++ b/posix/tst-spawn5.c > @@ -68,6 +68,8 @@ handle_restart (int argc, char *argv[]) > int fd; > _Bool found; > } *fds = xmalloc (sizeof (struct fd_t) * nfds); > + int maxfd = 0; > + int minfd = INT_MAX; > for (int i = 0; i < nfds; i++) > { > char *endptr; > @@ -77,6 +79,11 @@ handle_restart (int argc, char *argv[]) > > fds[i].fd = fd; > fds[i].found = false; > + > + if (fd < minfd) > + minfd = fd; > + if (fd > maxfd) > + maxfd = fd; > } I think this interferes with the objective of the test. You really need to pass lowfd to the target process. Thanks, Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-09 18:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-09 9:45 posix/tst-spawn5 failes under make -j Florian Weimer 2021-07-09 13:10 ` Adhemerval Zanella 2021-07-09 13:41 ` Florian Weimer 2021-07-09 14:17 ` Adhemerval Zanella 2021-07-09 18:40 ` Florian Weimer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).