Use dup2() to make message queue tests less flaky.

We're trying to test situations where a file descriptor number
gets reused after a file has been closed.  For that, it's better
to use dup2() to explicitly simulate this situation rather than
relying on timing and luck.

Change-Id: If86cdbe4246a7a1b9eeddd1c32a76559c4cf09c6
diff --git a/tests/tests/os/src/android/os/cts/MessageQueueTest.java b/tests/tests/os/src/android/os/cts/MessageQueueTest.java
index 8906c42..0c051bf 100644
--- a/tests/tests/os/src/android/os/cts/MessageQueueTest.java
+++ b/tests/tests/os/src/android/os/cts/MessageQueueTest.java
@@ -25,6 +25,8 @@
 import android.os.ParcelFileDescriptor;
 import android.os.ParcelFileDescriptor.AutoCloseInputStream;
 import android.os.ParcelFileDescriptor.AutoCloseOutputStream;
+import android.system.ErrnoException;
+import android.system.Os;
 import android.os.SystemClock;
 import android.os.MessageQueue.IdleHandler;
 import android.test.AndroidTestCase;
@@ -66,7 +68,7 @@
         }
     }
 
-    private enum Test {ADD_IDLE_HANDLER, REMOVE_IDLE_HANDLER};
+    private enum Test {ADD_IDLE_HANDLER, REMOVE_IDLE_HANDLER}
 
     /**
      * {@link HandlerThread} that adds or removes an idle handler depending on the {@link Test}
@@ -91,6 +93,7 @@
             super.onLooperPrepared();
 
             IdleHandler idleHandler = new IdleHandler() {
+                @Override
                 public boolean queueIdle() {
                     mIdleLatch.countDown();
                     return false;
@@ -168,6 +171,7 @@
     public void testMessageOrder() throws Exception {
 
         OrderTestHelper tester = new OrderTestHelper() {
+            @Override
             public void init() {
                 super.init();
                 long now = SystemClock.uptimeMillis() + 200;
@@ -191,6 +195,7 @@
 
         OrderTestHelper tester = new OrderTestHelper() {
 
+            @Override
             public void init() {
                 super.init();
                 long now = SystemClock.uptimeMillis() + 200;
@@ -200,6 +205,7 @@
                 mHandler.sendMessageAtFrontOfQueue(mHandler.obtainMessage(0));
             }
 
+            @Override
             public void handleMessage(Message msg) {
                 super.handleMessage(msg);
                 if (msg.what == 0) {
@@ -386,9 +392,9 @@
      * edge cases around closing file descriptors within the callback and adding
      * new ones with the same number.
      *
-     * Register a file descriptor, close it from within the callback before
-     * returning, return.  Then create a new file descriptor (with the same number),
-     * register it.  Ensure that we start getting events for the new file descriptor.
+     * Register a file descriptor, close it from within the callback, then return.
+     * Later, create a new file descriptor register it.  Ensure that we start getting
+     * events for the new file descriptor.
      *
      * This test exercises special logic in Looper.cpp for EPOLL_CTL_DEL handling EBADF.
      */
@@ -401,23 +407,19 @@
             final Handler handler = new Handler(thread.getLooper());
 
             final ParcelFileDescriptor[] pipe = ParcelFileDescriptor.createPipe();
-            final int oldReaderFd = pipe[0].getFd();
             try (final FileInputStream reader = new AutoCloseInputStream(pipe[0]);
                     final FileOutputStream writer = new AutoCloseOutputStream(pipe[1])) {
                 // Register the callback.
-                final boolean[] awoke = new boolean[1];
+                final CountDownLatch awoke = new CountDownLatch(1);
                 queue.addOnFileDescriptorEventListener(reader.getFD(),
-                        OnFileDescriptorEventListener.EVENT_ERROR, new OnFileDescriptorEventListener() {
+                        OnFileDescriptorEventListener.EVENT_ERROR,
+                        new OnFileDescriptorEventListener() {
                     @Override
                     public int onFileDescriptorEvents(FileDescriptor fd, int events) {
-                        awoke[0] = true;
+                        awoke.countDown();
 
-                        // Close the file descriptor before we return.
-                        try {
-                            reader.close();
-                        } catch (IOException ex) {
-                            throw new RuntimeException(ex);
-                        }
+                        // Close the reader before we return.
+                        closeQuietly(reader);
 
                         // Return 0 to unregister the callback.
                         return 0;
@@ -428,26 +430,23 @@
                 writer.close();
 
                 // Wait for the looper to catch up and run the callback.
+                assertTrue("awoke", awoke.await(TIMEOUT, TimeUnit.MILLISECONDS));
                 syncWait(handler);
-                assertTrue(awoke[0]);
             }
 
             // At this point, the reader and writer are both closed.
-            // If we're lucky, we can create a new pipe with the same file
-            // descriptor numbers as before.
+            // Make a new pipe and ensure that things still work as expected.
             final ParcelFileDescriptor[] pipe2 = ParcelFileDescriptor.createPipe();
-            assertEquals("Expected new pipe to be created with same fd number as "
-                    + "previous pipe we just closed for the purpose of this test.",
-                    oldReaderFd, pipe2[0].getFd());
             try (final FileInputStream reader2 = new AutoCloseInputStream(pipe2[0]);
                     final FileOutputStream writer2 = new AutoCloseOutputStream(pipe2[1])) {
                 // Register the callback.
-                final boolean[] awoke = new boolean[1];
+                final CountDownLatch awoke = new CountDownLatch(1);
                 queue.addOnFileDescriptorEventListener(reader2.getFD(),
-                        OnFileDescriptorEventListener.EVENT_INPUT, new OnFileDescriptorEventListener() {
+                        OnFileDescriptorEventListener.EVENT_INPUT,
+                        new OnFileDescriptorEventListener() {
                     @Override
                     public int onFileDescriptorEvents(FileDescriptor fd, int events) {
-                        awoke[0] = true;
+                        awoke.countDown();
 
                         // Return 0 to unregister the callback.
                         return 0;
@@ -458,8 +457,8 @@
                 writer2.close();
 
                 // Wait for the looper to catch up and run the callback.
+                assertTrue("awoke", awoke.await(TIMEOUT, TimeUnit.MILLISECONDS));
                 syncWait(handler);
-                assertTrue(awoke[0]);
             }
         } finally {
             thread.quitAndRethrow();
@@ -471,10 +470,10 @@
      * edge cases around closing file descriptors within the callback and adding
      * new ones with the same number.
      *
-     * Register a file descriptor, close it from within the callback before
-     * returning, create a new file descriptor (with the same number) and return.
-     * Then register the same file descriptor.  Ensure that we start getting events for
-     * the new file descriptor.
+     * Register a file descriptor, close it from within the callback, reassign its
+     * number to a different pipe, then return.  Later, register the same file descriptor
+     * again (now referring to a new pipe).  Ensure that we start getting
+     * events for the new file descriptor.
      *
      * This test exercises special logic in Looper.cpp for EPOLL_CTL_DEL handling ENOENT.
      */
@@ -487,69 +486,67 @@
             final Handler handler = new Handler(thread.getLooper());
 
             final ParcelFileDescriptor[] pipe = ParcelFileDescriptor.createPipe();
-            final int oldReaderFd = pipe[0].getFd();
-            try (final FileInputStream reader = new AutoCloseInputStream(pipe[0]);
-                    final FileOutputStream writer = new AutoCloseOutputStream(pipe[1])) {
-                // Register the callback.
-                final boolean[] awoke = new boolean[1];
-                queue.addOnFileDescriptorEventListener(reader.getFD(),
-                        OnFileDescriptorEventListener.EVENT_ERROR, new OnFileDescriptorEventListener() {
-                    @Override
-                    public int onFileDescriptorEvents(FileDescriptor fd, int events) {
-                        awoke[0] = true;
+            final ParcelFileDescriptor[] pipe2 = ParcelFileDescriptor.createPipe();
+            try {
+                final int oldReaderFd = pipe[0].getFd();
+                try (final FileInputStream reader = new AutoCloseInputStream(pipe[0]);
+                        final FileOutputStream writer = new AutoCloseOutputStream(pipe[1])) {
+                    // Register the callback.
+                    final CountDownLatch awoke = new CountDownLatch(1);
+                    queue.addOnFileDescriptorEventListener(reader.getFD(),
+                            OnFileDescriptorEventListener.EVENT_ERROR,
+                            new OnFileDescriptorEventListener() {
+                        @Override
+                        public int onFileDescriptorEvents(FileDescriptor fd, int events) {
+                            awoke.countDown();
 
-                        try {
-                            // Close the file descriptor before we return.
-                            reader.close();
+                            // Close the reader before we return and hijack its fd.
+                            hijackFd(pipe2, pipe);
 
-                            // At this point, the reader and writer are both closed.
-                            // Assuming no one else has created a file descriptor in the meantime,
-                            // when we recreate the pipe we will get the same number as before.
-                            final ParcelFileDescriptor[] pipe2 = ParcelFileDescriptor.createPipe();
-                            assertEquals("Expected new pipe to be created with same fd number as "
-                                    + "previous pipe we just closed for the purpose of this test.",
-                                    oldReaderFd, pipe2[0].getFd());
-                            pipe[0] = pipe2[0];
-                            pipe[1] = pipe2[1];
-                        } catch (IOException ex) {
-                            throw new RuntimeException(ex);
+                            // Return 0 to unregister the callback.
+                            return 0;
                         }
+                    });
 
-                        // Return 0 to unregister the callback.
-                        return 0;
-                    }
-                });
+                    // Close the writer to wake up the callback (due to hangup).
+                    writer.close();
 
-                // Close the writer to wake up the callback (due to hangup).
-                writer.close();
+                    // Wait for the looper to catch up and run the callback.
+                    assertTrue("awoke", awoke.await(TIMEOUT, TimeUnit.MILLISECONDS));
+                    syncWait(handler);
+                }
 
-                // Wait for the looper to catch up and run the callback.
-                syncWait(handler);
-                assertTrue(awoke[0]);
-            }
+                // Now we have a new pipe with the same file descriptor, make sure we can
+                // register it successfully.
+                assertEquals(oldReaderFd, pipe2[0].getFd());
+                try (final FileInputStream reader2 = new AutoCloseInputStream(pipe2[0]);
+                        final FileOutputStream writer2 = new AutoCloseOutputStream(pipe2[1])) {
+                    // Register the callback.
+                    final CountDownLatch awoke = new CountDownLatch(1);
+                    queue.addOnFileDescriptorEventListener(reader2.getFD(),
+                            OnFileDescriptorEventListener.EVENT_INPUT,
+                            new OnFileDescriptorEventListener() {
+                        @Override
+                        public int onFileDescriptorEvents(FileDescriptor fd, int events) {
+                            awoke.countDown();
 
-            // Now we have a new pipe, make sure we can register it successfully.
-            try (final FileInputStream reader2 = new AutoCloseInputStream(pipe[0]);
-                    final FileOutputStream writer2 = new AutoCloseOutputStream(pipe[1])) {
-                // Register the callback.
-                final boolean[] awoke = new boolean[1];
-                queue.addOnFileDescriptorEventListener(reader2.getFD(),
-                        OnFileDescriptorEventListener.EVENT_INPUT, new OnFileDescriptorEventListener() {
-                    @Override
-                    public int onFileDescriptorEvents(FileDescriptor fd, int events) {
-                        awoke[0] = true;
+                            // Return 0 to unregister the callback.
+                            return 0;
+                        }
+                    });
 
-                        // Return 0 to unregister the callback.
-                        return 0;
-                    }
-                });
+                    // Close the writer to wake up the callback (due to hangup).
+                    writer2.close();
 
-                // Close the writer to wake up the callback (due to hangup).
-                writer2.close();
-
-                // Wait for the looper to catch up and run the callback.
-                syncWait(handler);
-                assertTrue(awoke[0]);
+                    // Wait for the looper to catch up and run the callback.
+                    assertTrue("awoke", awoke.await(TIMEOUT, TimeUnit.MILLISECONDS));
+                    syncWait(handler);
+                }
+            } finally {
+                closeQuietly(pipe[0]);
+                closeQuietly(pipe[1]);
+                closeQuietly(pipe2[0]);
+                closeQuietly(pipe2[1]);
             }
         } finally {
             thread.quitAndRethrow();
@@ -561,10 +558,9 @@
      * edge cases around closing file descriptors within the callback and adding
      * new ones with the same number.
      *
-     * Register a file descriptor, close it from within the callback before
-     * returning, create a new file descriptor (with the same number),
-     * register it, and return.  Ensure that we start getting events for the
-     * new file descriptor.
+     * Register a file descriptor, close it from within the callback, reassign its
+     * number to a different pipe, register it, then return.
+     * Ensure that we start getting events for the new file descriptor.
      *
      * This test exercises special logic in Looper.cpp for EPOLL_CTL_MOD handling
      * ENOENT and fallback to EPOLL_CTL_ADD as well as sequence number checks when removing
@@ -579,71 +575,66 @@
             final Handler handler = new Handler(thread.getLooper());
 
             final ParcelFileDescriptor[] pipe = ParcelFileDescriptor.createPipe();
-            final boolean[] awoke2 = new boolean[1];
-            final int oldReaderFd = pipe[0].getFd();
-            try (final FileInputStream reader = new AutoCloseInputStream(pipe[0]);
-                    final FileOutputStream writer = new AutoCloseOutputStream(pipe[1])) {
-                // Register the callback.
-                final boolean[] awoke = new boolean[1];
-                queue.addOnFileDescriptorEventListener(reader.getFD(),
-                        OnFileDescriptorEventListener.EVENT_ERROR, new OnFileDescriptorEventListener() {
-                    @Override
-                    public int onFileDescriptorEvents(FileDescriptor fd, int events) {
-                        awoke[0] = true;
+            final ParcelFileDescriptor[] pipe2 = ParcelFileDescriptor.createPipe();
+            try {
+                final CountDownLatch awoke2 = new CountDownLatch(1);
+                final int oldReaderFd = pipe[0].getFd();
+                try (final FileInputStream reader = new AutoCloseInputStream(pipe[0]);
+                        final FileOutputStream writer = new AutoCloseOutputStream(pipe[1])) {
+                    // Register the callback.
+                    final CountDownLatch awoke = new CountDownLatch(1);
+                    queue.addOnFileDescriptorEventListener(reader.getFD(),
+                            OnFileDescriptorEventListener.EVENT_ERROR,
+                            new OnFileDescriptorEventListener() {
+                        @Override
+                        public int onFileDescriptorEvents(FileDescriptor fd, int events) {
+                            awoke.countDown();
 
-                        final ParcelFileDescriptor[] pipe2;
-                        try {
-                            // Close the file descriptor before we return.
-                            reader.close();
+                            // Close the reader before we return and hijack its fd.
+                            hijackFd(pipe2, pipe);
 
-                            // At this point, the reader and writer are both closed.
-                            // Assuming no one else has created a file descriptor in the meantime,
-                            // when we recreate the pipe we will get the same number as before.
-                            pipe2 = ParcelFileDescriptor.createPipe();
-                            assertEquals("Expected new pipe to be created with same fd number as "
-                                    + "previous pipe we just closed for the purpose of this test.",
-                                    oldReaderFd, pipe2[0].getFd());
-                            pipe[0] = pipe2[0];
-                            pipe[1] = pipe2[1];
-                        } catch (IOException ex) {
-                            throw new RuntimeException(ex);
+                            // Now we have a new pipe, make sure we can register it successfully.
+                            queue.addOnFileDescriptorEventListener(pipe2[0].getFileDescriptor(),
+                                    OnFileDescriptorEventListener.EVENT_INPUT,
+                                    new OnFileDescriptorEventListener() {
+                                @Override
+                                public int onFileDescriptorEvents(FileDescriptor fd, int events) {
+                                    awoke2.countDown();
+
+                                    // Return 0 to unregister the callback.
+                                    return 0;
+                                }
+                            });
+
+                            // Return 0 to unregister the callback.
+                            return 0;
                         }
+                    });
 
-                        // Now we have a new pipe, make sure we can register it successfully.
-                        queue.addOnFileDescriptorEventListener(pipe[0].getFileDescriptor(),
-                                OnFileDescriptorEventListener.EVENT_INPUT,
-                                new OnFileDescriptorEventListener() {
-                            @Override
-                            public int onFileDescriptorEvents(FileDescriptor fd, int events) {
-                                awoke2[0] = true;
+                    // Close the writer to wake up the callback (due to hangup).
+                    writer.close();
 
-                                // Return 0 to unregister the callback.
-                                return 0;
-                            }
-                        });
+                    // Wait for the looper to catch up and run the callback.
+                    assertTrue("awoke", awoke.await(TIMEOUT, TimeUnit.MILLISECONDS));
+                    syncWait(handler);
+                }
 
-                        // Return 0 to unregister the callback.
-                        return 0;
-                    }
-                });
-
-                // Close the writer to wake up the callback (due to hangup).
-                writer.close();
+                // Close the second writer to wake up the second callback (due to hangup).
+                pipe2[1].close();
 
                 // Wait for the looper to catch up and run the callback.
+                assertTrue("awoke2", awoke2.await(TIMEOUT, TimeUnit.MILLISECONDS));
                 syncWait(handler);
-                assertTrue(awoke[0]);
+
+                // Close the second reader now that we're done with the test.
+                assertEquals(oldReaderFd, pipe2[0].getFd());
+                pipe2[0].close();
+            } finally {
+                closeQuietly(pipe[0]);
+                closeQuietly(pipe[1]);
+                closeQuietly(pipe2[0]);
+                closeQuietly(pipe2[1]);
             }
-
-            // Close the second writer to wake up the second callback (due to hangup).
-            pipe[1].close();
-
-            // Wait for the looper to catch up and run the callback.
-            syncWait(handler);
-            assertTrue(awoke2[0]);
-
-            // Close the second reader now that we're done with the test.
-            pipe[0].close();
         } finally {
             thread.quitAndRethrow();
         }
@@ -655,7 +646,7 @@
      * new ones with the same number.
      *
      * Register a file descriptor, make a duplicate of it, close it from within the
-     * callback before returning, return.  Look for signs that the Looper is spinning
+     * callback, then return.  Look for signs that the Looper is spinning
      * and never getting a chance to block.
      *
      * This test exercises special logic in Looper.cpp for rebuilding the epoll set
@@ -676,12 +667,12 @@
                 try (final FileInputStream reader = new AutoCloseInputStream(pipe[0]);
                         final FileOutputStream writer = new AutoCloseOutputStream(pipe[1])) {
                     // Register the callback.
-                    final boolean[] awoke = new boolean[1];
+                    final CountDownLatch awoke = new CountDownLatch(1);
                     queue.addOnFileDescriptorEventListener(reader.getFD(),
                             OnFileDescriptorEventListener.EVENT_ERROR, new OnFileDescriptorEventListener() {
                         @Override
                         public int onFileDescriptorEvents(FileDescriptor fd, int events) {
-                            awoke[0] = true;
+                            awoke.countDown();
 
                             // Close the file descriptor before we return.
                             try {
@@ -699,8 +690,8 @@
                     writer.close();
 
                     // Wait for the looper to catch up and run the callback.
+                    assertTrue("awoke", awoke.await(TIMEOUT, TimeUnit.MILLISECONDS));
                     syncWait(handler);
-                    assertTrue(awoke[0]);
                 }
 
                 // Wait a little bit before we stop the thread.
@@ -729,6 +720,7 @@
             private int mBarrierToken1;
             private int mBarrierToken2;
 
+            @Override
             public void init() {
                 super.init();
                 mLastMessage = 10;
@@ -741,6 +733,7 @@
                 mHandler.sendEmptyMessage(6);
             }
 
+            @Override
             public void handleMessage(Message msg) {
                 super.handleMessage(msg);
                 if (msg.what == 3) {
@@ -797,7 +790,35 @@
                 latch.countDown();
             }
         });
-        latch.await(TIMEOUT, TimeUnit.MILLISECONDS);
+        assertTrue("Handler got stuck.", latch.await(TIMEOUT, TimeUnit.MILLISECONDS));
+    }
+
+    private static void closeQuietly(AutoCloseable c) {
+        if (c != null) {
+            try {
+                c.close();
+            } catch (RuntimeException rethrown) {
+                throw rethrown;
+            } catch (Exception ex) {
+            }
+        }
+    }
+
+    private static void hijackFd(ParcelFileDescriptor[] newPipe, ParcelFileDescriptor[] oldPipe) {
+        // Detach the old pipe's first fd and get its number.
+        int fd = oldPipe[0].detachFd();
+
+        // Assign the new pipe's first fd to the same number as the old pipe's first fd.
+        // This causes the old pipe's first fd to be closed and reassigned.
+        try {
+            Os.dup2(newPipe[0].getFileDescriptor(), fd);
+        } catch (ErrnoException ex) {
+            throw new RuntimeException(ex);
+        }
+
+        // Fix up the new pipe's first fd object.
+        closeQuietly(newPipe[0]);
+        newPipe[0] = ParcelFileDescriptor.adoptFd(fd);
     }
 
     /**
@@ -814,6 +835,7 @@
 
         public void init() {
             mHandler = new Handler() {
+                @Override
                 public void handleMessage(Message msg) {
                     OrderTestHelper.this.handleMessage(msg);
                 }
@@ -863,6 +885,7 @@
                 super("MessengerLooperThread");
             }
 
+            @Override
             public void onLooperPrepared() {
                 init();
                 mLooper = getLooper();
@@ -908,7 +931,7 @@
      * A HandlerThread that propagates exceptions out of the event loop
      * instead of crashing the process.
      */
-    private class AssertableHandlerThread extends HandlerThread {
+    private static class AssertableHandlerThread extends HandlerThread {
         private Throwable mThrowable;
         private long mRuntime;