am 3685db7f: am e8605af5: Merge "Avoid leaking file descriptors when returning drop box events." into froyo

Merge commit '3685db7f5dd8a830a4e096404d4924e12697fb78' into gingerbread-plus-aosp

* commit '3685db7f5dd8a830a4e096404d4924e12697fb78':
  Avoid leaking file descriptors when returning drop box events.

core/java/android/os/DropBoxManager.java

@@ -53,6 +53,9 @@ public class DropBoxManager {
     /** Flag value: Content can be decompressed with {@link java.util.zip.GZIPOutputStream}. */
     public static final int IS_GZIPPED = 4;
 
+    /** Flag value for serialization only: Value is a byte array, not a file descriptor */
+    private static final int HAS_BYTE_ARRAY = 8;
+
     /**
      * A single entry retrieved from the drop box.
      * This may include a reference to a stream, so you must call
@@ -68,12 +71,25 @@ public class DropBoxManager {
 
         /** Create a new empty Entry with no contents. */
         public Entry(String tag, long millis) {
-            this(tag, millis, (Object) null, IS_EMPTY);
+            if (tag == null) throw new NullPointerException("tag == null");
+
+            mTag = tag;
+            mTimeMillis = millis;
+            mData = null;
+            mFileDescriptor = null;
+            mFlags = IS_EMPTY;
         }
 
         /** Create a new Entry with plain text contents. */
         public Entry(String tag, long millis, String text) {
-            this(tag, millis, (Object) text.getBytes(), IS_TEXT);
+            if (tag == null) throw new NullPointerException("tag == null");
+            if (text == null) throw new NullPointerException("text == null");
+
+            mTag = tag;
+            mTimeMillis = millis;
+            mData = text.getBytes();
+            mFileDescriptor = null;
+            mFlags = IS_TEXT;
         }
 
         /**
@@ -81,7 +97,16 @@ public class DropBoxManager {
          * The data array must not be modified after creating this entry.
          */
         public Entry(String tag, long millis, byte[] data, int flags) {
-            this(tag, millis, (Object) data, flags);
+            if (tag == null) throw new NullPointerException("tag == null");
+            if (((flags & IS_EMPTY) != 0) != (data == null)) {
+                throw new IllegalArgumentException("Bad flags: " + flags);
+            }
+
+            mTag = tag;
+            mTimeMillis = millis;
+            mData = data;
+            mFileDescriptor = null;
+            mFlags = flags;
         }
 
         /**
@@ -89,7 +114,16 @@ public class DropBoxManager {
          * Takes ownership of the ParcelFileDescriptor.
          */
         public Entry(String tag, long millis, ParcelFileDescriptor data, int flags) {
-            this(tag, millis, (Object) data, flags);
+            if (tag == null) throw new NullPointerException("tag == null");
+            if (((flags & IS_EMPTY) != 0) != (data == null)) {
+                throw new IllegalArgumentException("Bad flags: " + flags);
+            }
+
+            mTag = tag;
+            mTimeMillis = millis;
+            mData = null;
+            mFileDescriptor = data;
+            mFlags = flags;
         }
 
         /**
@@ -97,31 +131,14 @@ public class DropBoxManager {
          * The file will be read when the entry's contents are requested.
          */
         public Entry(String tag, long millis, File data, int flags) throws IOException {
-            this(tag, millis, (Object) ParcelFileDescriptor.open(
+            if (tag == null) throw new NullPointerException("tag == null");
-                    data, ParcelFileDescriptor.MODE_READ_ONLY), flags);
+            if ((flags & IS_EMPTY) != 0) throw new IllegalArgumentException("Bad flags: " + flags);
-        }
-
-        /** Internal constructor for CREATOR.createFromParcel(). */
-        private Entry(String tag, long millis, Object value, int flags) {
-            if (tag == null) throw new NullPointerException();
-            if (((flags & IS_EMPTY) != 0) != (value == null)) throw new IllegalArgumentException();
 
             mTag = tag;
             mTimeMillis = millis;
+            mData = null;
+            mFileDescriptor = ParcelFileDescriptor.open(data, ParcelFileDescriptor.MODE_READ_ONLY);
             mFlags = flags;
-
-            if (value == null) {
-                mData = null;
-                mFileDescriptor = null;
-            } else if (value instanceof byte[]) {
-                mData = (byte[]) value;
-                mFileDescriptor = null;
-            } else if (value instanceof ParcelFileDescriptor) {
-                mData = null;
-                mFileDescriptor = (ParcelFileDescriptor) value;
-            } else {
-                throw new IllegalArgumentException();
-            }
         }
 
         /** Close the input stream associated with this entry. */
@@ -149,6 +166,7 @@ public class DropBoxManager {
             InputStream is = null;
             try {
                 is = getInputStream();
+                if (is == null) return null;
                 byte[] buf = new byte[maxBytes];
                 return new String(buf, 0, Math.max(0, is.read(buf)));
             } catch (IOException e) {
@@ -174,8 +192,14 @@ public class DropBoxManager {
         public static final Parcelable.Creator<Entry> CREATOR = new Parcelable.Creator() {
             public Entry[] newArray(int size) { return new Entry[size]; }
             public Entry createFromParcel(Parcel in) {
-                return new Entry(
+                String tag = in.readString();
-                        in.readString(), in.readLong(), in.readValue(null), in.readInt());
+                long millis = in.readLong();
+                int flags = in.readInt();
+                if ((flags & HAS_BYTE_ARRAY) != 0) {
+                    return new Entry(tag, millis, in.createByteArray(), flags & ~HAS_BYTE_ARRAY);
+                } else {
+                    return new Entry(tag, millis, in.readFileDescriptor(), flags);
+                }
             }
         };
 
@@ -187,11 +211,12 @@ public class DropBoxManager {
             out.writeString(mTag);
             out.writeLong(mTimeMillis);
             if (mFileDescriptor != null) {
-                out.writeValue(mFileDescriptor);
+                out.writeInt(mFlags & ~HAS_BYTE_ARRAY);  // Clear bit just to be safe
+                mFileDescriptor.writeToParcel(out, flags);
             } else {
-                out.writeValue(mData);
+                out.writeInt(mFlags | HAS_BYTE_ARRAY);
+                out.writeByteArray(mData);
             }
-            out.writeInt(mFlags);
         }
     }
 
@@ -225,7 +250,7 @@ public class DropBoxManager {
      * @param flags describing the data
      */
     public void addData(String tag, byte[] data, int flags) {
-        if (data == null) throw new NullPointerException();
+        if (data == null) throw new NullPointerException("data == null");
         try { mService.add(new Entry(tag, 0, data, flags)); } catch (RemoteException e) {}
     }
 
@@ -239,7 +264,7 @@ public class DropBoxManager {
      * @throws IOException if the file can't be opened
      */
     public void addFile(String tag, File file, int flags) throws IOException {
-        if (file == null) throw new NullPointerException();
+        if (file == null) throw new NullPointerException("file == null");
         Entry entry = new Entry(tag, 0, file, flags);
         try {
             mService.add(entry);

services/tests/servicestests/src/com/android/server/DropBoxTest.java

@@ -20,6 +20,10 @@ import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
 import android.os.DropBoxManager;
+import android.os.Parcel;
+import android.os.Parcelable;
+import android.os.ParcelFileDescriptor;
+import android.os.Process;
 import android.os.ServiceManager;
 import android.os.StatFs;
 import android.provider.Settings;
@@ -27,10 +31,13 @@ import android.test.AndroidTestCase;
 
 import com.android.server.DropBoxManagerService;
 
+import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileWriter;
+import java.io.IOException;
 import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.util.Random;
 import java.util.zip.GZIPOutputStream;
 
@@ -531,6 +538,203 @@ public class DropBoxTest extends AndroidTestCase {
         service.stop();
     }
 
+    public void testDropBoxEntrySerialization() throws Exception {
+        // Make sure DropBoxManager.Entry can be serialized to a Parcel and back
+        // under a variety of conditions.
+
+        Parcel parcel = Parcel.obtain();
+        File dir = getEmptyDir("testDropBoxEntrySerialization");
+
+        new DropBoxManager.Entry("empty", 1000000).writeToParcel(parcel, 0);
+        new DropBoxManager.Entry("string", 2000000, "String Value").writeToParcel(parcel, 0);
+        new DropBoxManager.Entry("bytes", 3000000, "Bytes Value".getBytes(),
+                DropBoxManager.IS_TEXT).writeToParcel(parcel, 0);
+        new DropBoxManager.Entry("zerobytes", 4000000, new byte[0], 0).writeToParcel(parcel, 0);
+        new DropBoxManager.Entry("emptybytes", 5000000, (byte[]) null,
+                DropBoxManager.IS_EMPTY).writeToParcel(parcel, 0);
+
+        try {
+            new DropBoxManager.Entry("badbytes", 99999,
+                    "Bad Bytes Value".getBytes(),
+                    DropBoxManager.IS_EMPTY).writeToParcel(parcel, 0);
+            fail("IllegalArgumentException expected for non-null byte[] and IS_EMPTY flags");
+        } catch (IllegalArgumentException e) {
+            // expected
+        }
+
+        try {
+            new DropBoxManager.Entry("badbytes", 99999, (byte[]) null, 0).writeToParcel(parcel, 0);
+            fail("IllegalArgumentException expected for null byte[] and non-IS_EMPTY flags");
+        } catch (IllegalArgumentException e) {
+            // expected
+        }
+
+        File f = new File(dir, "file.dat");
+        FileOutputStream os = new FileOutputStream(f);
+        os.write("File Value".getBytes());
+        os.close();
+
+        new DropBoxManager.Entry("file", 6000000, f, DropBoxManager.IS_TEXT).writeToParcel(
+                parcel, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+        new DropBoxManager.Entry("binfile", 7000000, f, 0).writeToParcel(
+                parcel, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+        new DropBoxManager.Entry("emptyfile", 8000000, (ParcelFileDescriptor) null,
+                DropBoxManager.IS_EMPTY).writeToParcel(parcel, 0);
+
+        try {
+            new DropBoxManager.Entry("badfile", 99999, new File(dir, "nonexist.dat"), 0);
+            fail("IOException expected for nonexistent file");
+        } catch (IOException e) {
+            // expected
+        }
+
+        try {
+            new DropBoxManager.Entry("badfile", 99999, f, DropBoxManager.IS_EMPTY).writeToParcel(
+                    parcel, 0);
+            fail("IllegalArgumentException expected for non-null file and IS_EMPTY flags");
+        } catch (IllegalArgumentException e) {
+            // expected
+        }
+
+        try {
+            new DropBoxManager.Entry("badfile", 99999, (ParcelFileDescriptor) null, 0);
+            fail("IllegalArgumentException expected for null PFD and non-IS_EMPTY flags");
+        } catch (IllegalArgumentException e) {
+            // expected
+        }
+
+        File gz = new File(dir, "file.gz");
+        GZIPOutputStream gzout = new GZIPOutputStream(new FileOutputStream(gz));
+        gzout.write("Gzip File Value".getBytes());
+        gzout.close();
+
+        new DropBoxManager.Entry("gzipfile", 9000000, gz,
+                DropBoxManager.IS_TEXT | DropBoxManager.IS_GZIPPED).writeToParcel(
+                    parcel, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+        new DropBoxManager.Entry("gzipbinfile", 10000000, gz,
+                DropBoxManager.IS_GZIPPED).writeToParcel(
+                    parcel, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+
+        //
+        // Switch from writing to reading
+        //
+
+        parcel.setDataPosition(0);
+        DropBoxManager.Entry e;
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("empty", e.getTag());
+        assertEquals(1000000, e.getTimeMillis());
+        assertEquals(DropBoxManager.IS_EMPTY, e.getFlags());
+        assertEquals(null, e.getText(100));
+        assertEquals(null, e.getInputStream());
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("string", e.getTag());
+        assertEquals(2000000, e.getTimeMillis());
+        assertEquals(DropBoxManager.IS_TEXT, e.getFlags());
+        assertEquals("String Value", e.getText(100));
+        assertEquals("String Value",
+                new BufferedReader(new InputStreamReader(e.getInputStream())).readLine());
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("bytes", e.getTag());
+        assertEquals(3000000, e.getTimeMillis());
+        assertEquals(DropBoxManager.IS_TEXT, e.getFlags());
+        assertEquals("Bytes Value", e.getText(100));
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("zerobytes", e.getTag());
+        assertEquals(4000000, e.getTimeMillis());
+        assertEquals(0, e.getFlags());
+        assertEquals(null, e.getText(100));
+        assertEquals(null,
+                new BufferedReader(new InputStreamReader(e.getInputStream())).readLine());
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("emptybytes", e.getTag());
+        assertEquals(5000000, e.getTimeMillis());
+        assertEquals(DropBoxManager.IS_EMPTY, e.getFlags());
+        assertEquals(null, e.getText(100));
+        assertEquals(null, e.getInputStream());
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("file", e.getTag());
+        assertEquals(6000000, e.getTimeMillis());
+        assertEquals(DropBoxManager.IS_TEXT, e.getFlags());
+        assertEquals("File Value", e.getText(100));
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("binfile", e.getTag());
+        assertEquals(7000000, e.getTimeMillis());
+        assertEquals(0, e.getFlags());
+        assertEquals(null, e.getText(100));
+        assertEquals("File Value",
+                new BufferedReader(new InputStreamReader(e.getInputStream())).readLine());
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("emptyfile", e.getTag());
+        assertEquals(8000000, e.getTimeMillis());
+        assertEquals(DropBoxManager.IS_EMPTY, e.getFlags());
+        assertEquals(null, e.getText(100));
+        assertEquals(null, e.getInputStream());
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("gzipfile", e.getTag());
+        assertEquals(9000000, e.getTimeMillis());
+        assertEquals(DropBoxManager.IS_TEXT, e.getFlags());
+        assertEquals("Gzip File Value", e.getText(100));
+        e.close();
+
+        e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+        assertEquals("gzipbinfile", e.getTag());
+        assertEquals(10000000, e.getTimeMillis());
+        assertEquals(0, e.getFlags());
+        assertEquals(null, e.getText(100));
+        assertEquals("Gzip File Value",
+                new BufferedReader(new InputStreamReader(e.getInputStream())).readLine());
+        e.close();
+
+        assertEquals(0, parcel.dataAvail());
+        parcel.recycle();
+    }
+
+    public void testDropBoxEntrySerializationDoesntLeakFileDescriptors() throws Exception {
+        File dir = getEmptyDir("testDropBoxEntrySerialization");
+        File f = new File(dir, "file.dat");
+        FileOutputStream os = new FileOutputStream(f);
+        os.write("File Value".getBytes());
+        os.close();
+
+        int before = countOpenFiles();
+        assertTrue(before > 0);
+
+        for (int i = 0; i < 1000; i++) {
+            Parcel parcel = Parcel.obtain();
+            new DropBoxManager.Entry("file", 1000000, f, 0).writeToParcel(
+                    parcel, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+
+            parcel.setDataPosition(0);
+            DropBoxManager.Entry e = DropBoxManager.Entry.CREATOR.createFromParcel(parcel);
+            assertEquals("file", e.getTag());
+            e.close();
+
+            parcel.recycle();
+        }
+
+        int after = countOpenFiles();
+        assertTrue(after > 0);
+        assertTrue(after < before + 20);
+    }
+
     private void addRandomEntry(DropBoxManager dropbox, String tag, int size) throws Exception {
         byte[] bytes = new byte[size];
         new Random(System.currentTimeMillis()).nextBytes(bytes);
@@ -564,4 +768,8 @@ public class DropBoxTest extends AndroidTestCase {
         assertTrue(dir.listFiles().length == 0);
         return dir;
     }
+
+    private int countOpenFiles() {
+        return new File("/proc/" + Process.myPid() + "/fd").listFiles().length;
+    }
 }