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
This commit is contained in:
Zim
2021-07-29 20:51:10 +01:00
parent 9eee7d3f29
commit 366493edd2
4 changed files with 37 additions and 20 deletions

View File

@@ -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));
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;
}
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;

View File

@@ -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();
}
}
}

View File

@@ -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,11 +304,15 @@ public class MediaMetadataRetriever implements AutoCloseable {
*/
public void setDataSource(FileDescriptor fd, long offset, long length)
throws IllegalArgumentException {
ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fd);
if (modernFd == null) {
_setDataSource(fd, offset, length);
} else {
_setDataSource(modernFd.getFileDescriptor(), offset, length);
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);
}
}

View File

@@ -1271,11 +1271,14 @@ public class MediaPlayer extends PlayerBase
*/
public void setDataSource(FileDescriptor fd, long offset, long length)
throws IOException, IllegalArgumentException, IllegalStateException {
ParcelFileDescriptor modernFd = FileUtils.convertToModernFd(fd);
if (modernFd == null) {
_setDataSource(fd, offset, length);
} else {
_setDataSource(modernFd.getFileDescriptor(), offset, length);
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);
}
}