aboutsummaryrefslogtreecommitdiff
path: root/libbuild2/test/rule.cxx
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2022-12-19 18:00:22 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2023-01-09 14:05:36 +0300
commit618194207fc7d093cc9a353561700ab8f8684735 (patch)
tree579e529083dcffcff3a87bff24e46552ce878875 /libbuild2/test/rule.cxx
parent1457cdf3d7615f42d33246fb456bcb4332b35e6a (diff)
Fix simple and script tests to correctly terminate processes which don't close stderr on exit
Diffstat (limited to 'libbuild2/test/rule.cxx')
-rw-r--r--libbuild2/test/rule.cxx98
1 files changed, 84 insertions, 14 deletions
diff --git a/libbuild2/test/rule.cxx b/libbuild2/test/rule.cxx
index 0ee7641..2d02d39 100644
--- a/libbuild2/test/rule.cxx
+++ b/libbuild2/test/rule.cxx
@@ -668,6 +668,16 @@ namespace build2
//
bool terminated = false;
+ // True if this process has been terminated but we failed to read out
+ // its stderr stream in the reasonable timeframe (2 seconds) after the
+ // termination.
+ //
+ // Note that this may happen if there is a still running child process
+ // of the terminated process which has inherited the parent's stderr
+ // file descriptor.
+ //
+ bool unread_stderr = false;
+
pipe_process* prev; // NULL for the left-most program.
pipe_process* next; // Left-most program for the right-most program.
@@ -780,10 +790,14 @@ namespace build2
// Read out all the pipeline's buffered strerr streams watching for
// the deadline, if specified. If the deadline is reached, then
- // terminate the whole pipeline, reset the deadline to nullopt, and
- // continue reading. Note that the further reading will be performed
- // without timeout. This, however, is fine since all the processes are
- // terminated and we only need to read out the buffered data.
+ // terminate the whole pipeline, move the deadline by another 2
+ // seconds, and continue reading.
+ //
+ // Note that we assume that this timeout increment is normally
+ // sufficient to read out the buffered data written by the already
+ // terminated processes. If, however, that's not the case (see
+ // pipe_process for the possible reasons), then we just set
+ // unread_stderr flag to true for such processes and bail out.
//
// Also note that this implementation is inspired by the
// script::run_pipe::read_pipe() lambda.
@@ -800,6 +814,7 @@ namespace build2
}
optional<timestamp> dl (deadline);
+ bool terminated (false);
for (size_t unread (fds.size ()); unread != 0;)
{
@@ -814,9 +829,38 @@ namespace build2
if (*dl <= now || ifdselect (fds, *dl - now) == 0)
{
- term_pipe (&pp);
- dl = nullopt;
- continue;
+ if (!terminated)
+ {
+ term_pipe (&pp);
+ terminated = true;
+
+ dl = system_clock::now () + chrono::seconds (2);
+ continue;
+ }
+ else
+ {
+ for (fdselect_state& s: fds)
+ {
+ if (s.fd != nullfd)
+ {
+ pipe_process* p (static_cast<pipe_process*> (s.data));
+
+ p->unread_stderr = true;
+
+ // Let's also close the stderr stream not to confuse
+ // diag_buffer::close() (see script::read() for
+ // details).
+ //
+ try
+ {
+ p->dbuf.is.close ();
+ }
+ catch (const io_error&) {}
+ }
+ }
+
+ break;
+ }
}
}
else
@@ -869,9 +913,9 @@ namespace build2
// Iterate over the pipeline processes left to right, printing their
// stderr if buffered and issuing the diagnostics if the exit code is
- // not available (terminated abnormally or due to a deadline) or is
- // non-zero. Afterwards, fail if any of the processes didn't terminate
- // normally with zero code.
+ // not available (terminated abnormally or due to a deadline), is
+ // non-zero, or stderr was not fully read. Afterwards, fail if any of
+ // such a faulty processes were encountered.
//
// Note that we only issue diagnostics for the first failure.
//
@@ -920,18 +964,44 @@ namespace build2
{
diag_record dr;
- if (!pe || !pe->normal () || pe->code () != 0)
+ // Note that there can be a race, so that the process we have
+ // terminated due to reaching the deadline has in fact exited
+ // normally. Thus, the 'unread stderr' situation can also happen
+ // to a successfully terminated process. If that's the case, we
+ // report this problem as the main error and the secondary error
+ // otherwise.
+ //
+ if (!pe ||
+ !pe->normal () ||
+ pe->code () != 0 ||
+ p->unread_stderr)
{
fail = true;
dr << error << "test " << t << " failed" // Multi test: test 1.
<< error << "process " << p->args[0] << ' ';
- if (pe)
- dr << *pe;
- else
+ if (!pe)
+ {
dr << "terminated: execution timeout expired";
+ if (p->unread_stderr)
+ dr << error << "stderr not closed after exit";
+ }
+ else if (!pe->normal () || pe->code () != 0)
+ {
+ dr << *pe;
+
+ if (p->unread_stderr)
+ dr << error << "stderr not closed after exit";
+ }
+ else
+ {
+ assert (p->unread_stderr);
+
+ dr << "stderr not closed after exit";
+ }
+
if (verb == 1)
{
dr << info << "test command line: ";