RFR [8024521] (process) Async close issues with Process InputStream

[Martin Buchholz]

I include an improved test, that drops the repro time by an order of
magnitude, to around 20 seconds, but still too long and non-portable to
make this a well-behaved jtreg test.  I suggest checking in this version,
except that I would still make this a manual test.  Now that I have a
better repro, I can think about perhaps making a better fix.

/*
 * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2 only, as
 * published by the Free Software Foundation.
 *
 * This code is distributed in the hope that it will be useful, but WITHOUT
 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 * version 2 for more details (a copy is included in the LICENSE file that
 * accompanied this code).
 *
 * You should have received a copy of the GNU General Public License version
 * 2 along with this work; if not, write to the Free Software Foundation,
 * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
 *
 * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
 * or visit www.oracle.com if you need additional information or have any
 * questions.
 */

/**
 * @test
 * @bug 8024521
 * @summary Closing ProcessPipeInputStream at the time the process exits is
racy
 *          and leads to data corruption.
 * @run main/othervm/timeout=700 -Xmx8M CloseRace2
 */

import java.io.*;
import java.util.ArrayList;
import java.util.List;

public class CloseRace2 {
    // Linux only

    private static final String BIG_FILE = "bigfile";

    private static final int[] procFDs = new int[6];

    static boolean fdInUse(int i) {
        return new File("/proc/self/fd/" + i).exists();
    }

    static boolean[] procFDsInUse() {
        boolean[] inUse = new boolean[procFDs.length];
        for (int i = 0; i < procFDs.length; i++)
            inUse[i] = fdInUse(procFDs[i]);
        return inUse;
    }

    static int count(boolean[] bits) {
        int count = 0;
        for (int i = 0; i < bits.length; i++)
            count += bits[i] ? 1 : 0;
        return count;
    }

    public static void main(String args[]) throws Exception {
        if (!System.getProperty("os.name").equals("Linux"))
            return;

        // Catch Errors from process reaper
        Thread.setDefaultUncaughtExceptionHandler
            ((t, e) -> { e.printStackTrace(); System.exit(1); });

        try (RandomAccessFile f = new RandomAccessFile(BIG_FILE, "rw")) {
            f.setLength(Runtime.getRuntime().maxMemory()); // provoke OOME
        }

        for (int i = 0, j = 0; j < procFDs.length; i++)
            if (!fdInUse(i))
                procFDs[j++] = i;

        Thread[] threads = {
            new Thread(new OpenLoop()),
            new Thread(new ExecLoop()),
        };
        for (Thread thread : threads)
            thread.start();

        Thread.sleep(10 * 60 * 1000);

        for (Thread thread : threads)
            thread.interrupt();
        for (Thread thread : threads)
            thread.join();
    }

    static class OpenLoop implements Runnable {
        public void run() {
            while (!Thread.interrupted()) {
                try {
                    // wait for ExecLoop to finish creating process
                    do {} while (count(procFDsInUse()) != 3);
                    List<InputStream> iss = new ArrayList<>(3);
                    // eat up three "holes" (closed ends of pipe fd pairs)
                    for (int i = 0; i < 3; i++)
                        iss.add(new FileInputStream(BIG_FILE));
                    do {} while (count(procFDsInUse()) == procFDs.length);
                    // hopefully this will racily occupy empty fd slot
                    iss.add(new FileInputStream(BIG_FILE));
                    Thread.sleep(1); // Widen race window
                    for (InputStream is : iss)
                        is.close();
                } catch (InterruptedException e) {
                    break;
                } catch (Exception e) {
                    throw new Error(e);
                }
            }
        }
    }

    static class ExecLoop implements Runnable {
        public void run() {
            ProcessBuilder builder = new ProcessBuilder("/bin/true");
            while (!Thread.interrupted()) {
                try {
                    // wait for OpenLoop to finish
                    do {} while (count(procFDsInUse()) > 0);
                    Process process = builder.start();
                    InputStream is = process.getInputStream();
                    process.waitFor();
                    is.close();
                } catch (InterruptedException e) {
                    break;
                } catch (Exception e) {
                    throw new Error(e);
                }
            }
        }
    }
}



On Wed, Oct 23, 2013 at 5:05 AM, Alan Bateman <Alan.Bateman at oracle.com>wrote:

> On 23/10/2013 12:41, Ivan Gerasimov wrote:
>
>> Thank you Rob for offering the sponsor's help!
>> Here's the exported patch: http://cr.openjdk.java.net/~**
>> igerasim/2commit/8024521-jdk8-**Async-close-race.patch<http://cr.openjdk.java.net/~igerasim/2commit/8024521-jdk8-Async-close-race.patch>
>>
>> Alan, I reduced the default time gap to 20 seconds as you suggested.
>>
> Thanks for dialing it down. If Martin comes up with a test that duplicates
> it more efficiently then we can replace the test. From Matthias's mail to
> jdk8-dev [1] then there is a proposal to allow test stablization fixes to
> be pushed after ZBB so that gives us a window in which to fix test issues
> without needing special approvals.
>
> -Alan.
>
> [1] http://mail.openjdk.java.net/**pipermail/jdk8-dev/2013-**
> October/003443.html<http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-October/003443.html>
>