Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 366493ed authored by Zim's avatar Zim
Browse files

Fix fd leak while bypassing transcoding in media APIs 

To call the media service while bypassing transcoding there are 4 file
descriptors involved:

A: The original file descriptor the app owns and is responsible for
closing
B: Dupe of A, to pass into the MediaStore API as an input to receive a
non-transoding fd
C: The output from the MediaStore API
D: Final fd owned by the media service, after C gets duped over binder
as part of setDataSource

We were leaking (B) and (C). Now we close them appropriately.

See I0124ec8fbd0471237e99bab321f903544f8fe1f8 for another 2 fd leak
fix in MediaProvider

Test: atest TranscodeTest
Test: Manual with 'adb shell lsof | grep <filename>'
Bug: 194828489
Bug: 179804072
Change-Id: I978257bbc4a8f6813b6e6a5ce22124257204f432
Merged-In: I978257bbc4a8f6813b6e6a5ce22124257204f432
parent 9eee7d3f

core/java/android/os/FileUtils.java

+9 −9
Original line number Diff line number Diff line
@@ -1460,15 +1460,15 @@ public final class FileUtils { 
    /** {@hide} */

    @VisibleForTesting

    public static ParcelFileDescriptor convertToModernFd(FileDescriptor fd) {

        try {

        Context context = AppGlobals.getInitialApplication();

        if (UserHandle.getAppId(Process.myUid()) == getMediaProviderAppId(context)) {

            // Never convert modern fd for MediaProvider, because this requires

            // MediaStore#scanFile and can cause infinite loops when MediaProvider scans

            return null;

        }

            return MediaStore.getOriginalMediaFormatFileDescriptor(context,

                    ParcelFileDescriptor.dup(fd));



        try (ParcelFileDescriptor dupFd = ParcelFileDescriptor.dup(fd)) {

            return MediaStore.getOriginalMediaFormatFileDescriptor(context, dupFd);

        } catch (Exception e) {

            Log.d(TAG, "Failed to convert to modern format file descriptor", e);

            return null;

media/java/android/media/ExifInterface.java

+8 −1
Original line number Diff line number Diff line
@@ -1573,6 +1573,9 @@ public class ExifInterface { 
            if (isFdDuped) {

                closeFileDescriptor(fileDescriptor);

            }

            if (modernFd != null) {

                modernFd.close();

            }

        }

    }
@@ -2554,12 +2557,13 @@ public class ExifInterface { 


    private void initForFilename(String filename) throws IOException {

        FileInputStream in = null;

        ParcelFileDescriptor modernFd = null;

        mAssetInputStream = null;

        mFilename = filename;

        mIsInputStream = false;

        try {

            in = new FileInputStream(filename);

            ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(in.getFD());

            modernFd = FileUtils.convertToModernFd(in.getFD());

            if (modernFd != null) {

                closeQuietly(in);

                in = new FileInputStream(modernFd.getFileDescriptor());
@@ -2570,6 +2574,9 @@ public class ExifInterface { 
            loadAttributes(in);

        } finally {

            closeQuietly(in);

            if (modernFd != null) {

                modernFd.close();

            }

        }

    }

media/java/android/media/MediaMetadataRetriever.java

+12 −5
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import android.os.IBinder; 
import android.os.ParcelFileDescriptor;

import android.os.SystemProperties;

import android.text.TextUtils;

import android.util.Log;



import java.io.FileDescriptor;

import java.io.FileInputStream;
@@ -52,6 +53,8 @@ import java.util.Map; 
 * frame and meta data from an input media file.

 */

public class MediaMetadataRetriever implements AutoCloseable {

    private static final String TAG = "MediaMetadataRetriever";



    // borrowed from ExoPlayer

    private static final String[] STANDARD_GENRES = new String[] {

            // These are the official ID3v1 genres.
@@ -301,12 +304,16 @@ public class MediaMetadataRetriever implements AutoCloseable { 
     */

    public void setDataSource(FileDescriptor fd, long offset, long length)

            throws IllegalArgumentException  {

        ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fd);



        try (ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fd)) {

            if (modernFd == null) {

                _setDataSource(fd, offset, length);

            } else {

                _setDataSource(modernFd.getFileDescriptor(), offset, length);

            }

        } catch (IOException e) {

            Log.w(TAG, "Ignoring IO error while setting data source", e);

        }

    }



    private native void _setDataSource(FileDescriptor fd, long offset, long length)

media/java/android/media/MediaPlayer.java

+8 −5
Original line number Diff line number Diff line
@@ -1271,12 +1271,15 @@ public class MediaPlayer extends PlayerBase 
     */

    public void setDataSource(FileDescriptor fd, long offset, long length)

            throws IOException, IllegalArgumentException, IllegalStateException {

        ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fd);

        try (ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fd)) {

            if (modernFd == null) {

                _setDataSource(fd, offset, length);

            } else {

                _setDataSource(modernFd.getFileDescriptor(), offset, length);

            }

        } catch (IOException e) {

            Log.w(TAG, "Ignoring IO error while setting data source", e);

        }

    }



    private native void _setDataSource(FileDescriptor fd, long offset, long length)