diff options
3 files changed, 60 insertions, 37 deletions
diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java b/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java index ec7ba287acec..96bb5ef4e45e 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java @@ -44,7 +44,7 @@ import java.util.Objects; /** * Class for representing how a blob can be shared. * - * Note that this class is not thread-safe, callers need to take of synchronizing access. + * Note that this class is not thread-safe, callers need to take care of synchronizing access. */ class BlobAccessMode { @Retention(RetentionPolicy.SOURCE) diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java index cea7fcca6e20..03573d966cd0 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java @@ -61,6 +61,8 @@ import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.XmlUtils; import com.android.server.blob.BlobStoreManagerService.DumpArgs; +import libcore.io.IoUtils; + import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; import org.xmlpull.v1.XmlSerializer; @@ -349,14 +351,16 @@ class BlobMetadata { } catch (ErrnoException e) { throw e.rethrowAsIOException(); } - synchronized (mMetadataLock) { - return createRevocableFdLocked(fd, callingPackage); + try { + return createRevocableFd(fd, callingPackage); + } catch (IOException e) { + IoUtils.closeQuietly(fd); + throw e; } } - @GuardedBy("mMetadataLock") @NonNull - private ParcelFileDescriptor createRevocableFdLocked(FileDescriptor fd, + private ParcelFileDescriptor createRevocableFd(FileDescriptor fd, String callingPackage) throws IOException { final RevocableFileDescriptor revocableFd = new RevocableFileDescriptor(mContext, fd); diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java index 62701e52781d..7dc1bd089afb 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java @@ -57,6 +57,8 @@ import com.android.internal.util.XmlUtils; import com.android.server.blob.BlobStoreManagerService.DumpArgs; import com.android.server.blob.BlobStoreManagerService.SessionStateChangeListener; +import libcore.io.IoUtils; + import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; import org.xmlpull.v1.XmlSerializer; @@ -190,27 +192,37 @@ class BlobStoreSession extends IBlobStoreSession.Stub { throw new IllegalStateException("Not allowed to write in state: " + stateToString(mState)); } + } - try { - return openWriteLocked(offsetBytes, lengthBytes); - } catch (IOException e) { - throw ExceptionUtils.wrap(e); + FileDescriptor fd = null; + try { + fd = openWriteInternal(offsetBytes, lengthBytes); + final RevocableFileDescriptor revocableFd = new RevocableFileDescriptor(mContext, fd); + synchronized (mSessionLock) { + if (mState != STATE_OPENED) { + IoUtils.closeQuietly(fd); + throw new IllegalStateException("Not allowed to write in state: " + + stateToString(mState)); + } + trackRevocableFdLocked(revocableFd); + return revocableFd.getRevocableFileDescriptor(); } + } catch (IOException e) { + IoUtils.closeQuietly(fd); + throw ExceptionUtils.wrap(e); } } - @GuardedBy("mSessionLock") @NonNull - private ParcelFileDescriptor openWriteLocked(@BytesLong long offsetBytes, + private FileDescriptor openWriteInternal(@BytesLong long offsetBytes, @BytesLong long lengthBytes) throws IOException { // TODO: Add limit on active open sessions/writes/reads - FileDescriptor fd = null; try { final File sessionFile = getSessionFile(); if (sessionFile == null) { throw new IllegalStateException("Couldn't get the file for this session"); } - fd = Os.open(sessionFile.getPath(), O_CREAT | O_RDWR, 0600); + final FileDescriptor fd = Os.open(sessionFile.getPath(), O_CREAT | O_RDWR, 0600); if (offsetBytes > 0) { final long curOffset = Os.lseek(fd, offsetBytes, SEEK_SET); if (curOffset != offsetBytes) { @@ -221,10 +233,10 @@ class BlobStoreSession extends IBlobStoreSession.Stub { if (lengthBytes > 0) { mContext.getSystemService(StorageManager.class).allocateBytes(fd, lengthBytes); } + return fd; } catch (ErrnoException e) { - e.rethrowAsIOException(); + throw e.rethrowAsIOException(); } - return createRevocableFdLocked(fd); } @Override @@ -236,29 +248,40 @@ class BlobStoreSession extends IBlobStoreSession.Stub { throw new IllegalStateException("Not allowed to read in state: " + stateToString(mState)); } + } - try { - return openReadLocked(); - } catch (IOException e) { - throw ExceptionUtils.wrap(e); + FileDescriptor fd = null; + try { + fd = openReadInternal(); + final RevocableFileDescriptor revocableFd = new RevocableFileDescriptor(mContext, fd); + synchronized (mSessionLock) { + if (mState != STATE_OPENED) { + IoUtils.closeQuietly(fd); + throw new IllegalStateException("Not allowed to read in state: " + + stateToString(mState)); + } + trackRevocableFdLocked(revocableFd); + return revocableFd.getRevocableFileDescriptor(); } + + } catch (IOException e) { + IoUtils.closeQuietly(fd); + throw ExceptionUtils.wrap(e); } } - @GuardedBy("mSessionLock") @NonNull - private ParcelFileDescriptor openReadLocked() throws IOException { - FileDescriptor fd = null; + private FileDescriptor openReadInternal() throws IOException { try { final File sessionFile = getSessionFile(); if (sessionFile == null) { throw new IllegalStateException("Couldn't get the file for this session"); } - fd = Os.open(sessionFile.getPath(), O_RDONLY, 0); + final FileDescriptor fd = Os.open(sessionFile.getPath(), O_RDONLY, 0); + return fd; } catch (ErrnoException e) { - e.rethrowAsIOException(); + throw e.rethrowAsIOException(); } - return createRevocableFdLocked(fd); } @Override @@ -379,7 +402,7 @@ class BlobStoreSession extends IBlobStoreSession.Stub { } mState = state; - revokeAllFdsLocked(); + revokeAllFds(); if (sendCallback) { mListener.onStateChanged(this); @@ -415,20 +438,17 @@ class BlobStoreSession extends IBlobStoreSession.Stub { } } - @GuardedBy("mSessionLock") - private void revokeAllFdsLocked() { - for (int i = mRevocableFds.size() - 1; i >= 0; --i) { - mRevocableFds.get(i).revoke(); + private void revokeAllFds() { + synchronized (mRevocableFds) { + for (int i = mRevocableFds.size() - 1; i >= 0; --i) { + mRevocableFds.get(i).revoke(); + } + mRevocableFds.clear(); } - mRevocableFds.clear(); } @GuardedBy("mSessionLock") - @NonNull - private ParcelFileDescriptor createRevocableFdLocked(FileDescriptor fd) - throws IOException { - final RevocableFileDescriptor revocableFd = - new RevocableFileDescriptor(mContext, fd); + private void trackRevocableFdLocked(RevocableFileDescriptor revocableFd) { synchronized (mRevocableFds) { mRevocableFds.add(revocableFd); } @@ -437,7 +457,6 @@ class BlobStoreSession extends IBlobStoreSession.Stub { mRevocableFds.remove(revocableFd); } }); - return revocableFd.getRevocableFileDescriptor(); } @Nullable |