[Rt-commit] rt branch, 4.0/dbh-disconnect-after-safe-run-child, updated. rt-4.0.3-49-ga6f29f3

[Alex Vandiver]

The branch, 4.0/dbh-disconnect-after-safe-run-child has been updated
       via  a6f29f34f78bbe09e6417d521594f7b2dbfaed4b (commit)
      from  fa65b29c46def6414a7b54a250469cddac19b6e0 (commit)

Summary of changes:
 t/api/safe-run-child-util.t |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

- Log -----------------------------------------------------------------
commit a6f29f34f78bbe09e6417d521594f7b2dbfaed4b
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Dec 15 15:37:40 2011 -0500

    Use sleep() and signals to remove a possible race condition between processes
    
    It is possible that the parent could try to open the first file before
    the child has had a chance to write it, which would lead to a spurious
    test failure.  To avoid this, we establish a 3-second grace period
    during which we sleep().  Since any signal ends a sleep() call early, we
    will either be woken by a USR1 signal from the child when it is ready,
    or we will simply carry on and check for existence after 3 seconds.  We
    also install a trivial signal handler to allow us to skip the sleep()
    entirely if the signal arrives before we would enter the sleep().
    
    It is theoretically possible that on sufficiently heavily-loaded
    systems, 3 seconds is insufficient; if the system is that heavily
    loaded, however, you likely have other problems.

diff --git a/t/api/safe-run-child-util.t b/t/api/safe-run-child-util.t
index e94f14e..c4d2a8b 100644
--- a/t/api/safe-run-child-util.t
+++ b/t/api/safe-run-child-util.t
@@ -141,12 +141,20 @@ END
     require Time::HiRes;
     my $start = Time::HiRes::time();
     my $pid;
+
+    # Set up a poor man's semaphore
+    my $all_set = 0;
+    $SIG{USR1} = sub {$all_set++};
+
     my $res = safe_run_child {
         if ($pid = fork) { return 'parent' }
 
         open my $fh, '>', RT::Test->temp_directory .'/first';
         print $fh "child";
         close $fh;
+        # Signal that the first file is now all set; we need to do this
+        # to avoid a race condition
+        kill 10, getppid(); # USR1
 
         sleep 5;
 
@@ -157,6 +165,15 @@ END
         exit 0;
     };
     ok( Time::HiRes::time() - $start < 5, "Didn't wait until child finished" );
+
+    # Wait for up to 3 seconds to get signaled that the child has made
+    # the file (the USR1 will break out of the sleep()).  This _should_
+    # be immediate, but there's a race between the parent and child
+    # here, since there's no wait()'ing.  There's still a tiny race
+    # where the signal could come in betwene the $all_set check and the
+    # sleep, but that just means we sleep for 3 seconds uselessly.
+    sleep 3 unless $all_set;
+
     is $res, 'parent', "correct return value";
     is( RT::Test->file_content([RT::Test->temp_directory, 'first'], unlink => 1 ),
         'child',

-----------------------------------------------------------------------