Fix apparent race in SubprocessTest
Summary: The code originally contained the line EXPECT_EQ((rcount == lineCount), r); // EOF iff at lineCount Which asserted that, on the last iteration, EOF was being read (and thus r was true). This line has been causing errors in contbuild. There seem to be two issues at play. First, the left hand side of the EXPECT_EQ is always false. rcount is incremented three lines down, and the (implicit) loop ends when rcount equals lineCount. This means that, on the last iteration, rcount is actually one less than lineCount. Hence the check is effectively asserting that r is false. The second issue is that r is rarely true. Empirically, a necessary but not sufficient condition is that there be lots of other processes running at the same time (such as when running fbconfig -r folly && fbmake runtests_opt). This would explain why the test passes during development but fails in contbuild. I am guessing that, when there are few processes running, Subprocess is predictably ceasing to read before whichever other process yields EOF, and that this does not always happen when there is resource contention. As an immediate fix, I am asserting that r may only be true on the last iteration. Could someone (cc @tudorb) who is more familiar with the intricacies of Subprocess please check to see if this is an idiosyncrasy of the test, or if there is actually an underlying bug. Test Plan: Before the change, run fbconfig -r folly && fbmake runtests_opt repeatedly. Notice that most of the iterations yield an error from line 376 (which is consistent with contbuild error logs). After the change, notice that the error disappears. Reviewed By: andrewjcg@fb.com FB internal diff: D1269124
Showing
Please register or sign in to comment