Syncing FileDescriptor causes problems with closed streams [173037611]

Fixed

Bug

Status Update

No update yet.

Description

ho...@gmail.com

created issue #1

Nov 12, 2020 01:54AM

DataStore Component used: datastore-core

DataStore Version used: 1.0.0-alpha02

Devices/Android versions reproduced on: Emulator/Pixel 3, API 30

I've created a sample project that demonstrates the issue here: https://github.com/chris-horner/DataStore-Bug

I've written a custom Serializer that attempts to encrypt and decrypt the data written to disk. The problem I've run into is that I need to close the OutputStream provided in writeTo(), as I'm making use of Okio's CipherSink. Closing the stream is how CipherSink writes final data to the stream.

However closing OutputStream causes SingleProcessDataStore to throw a java.io.SyncFailedException when it attempts to invoke

stream.fd.sync()

with the stracktrace being

java.io.SyncFailedException: sync failed
    at java.io.FileDescriptor.sync(Native Method)
    at androidx.datastore.SingleProcessDataStore.writeData$datastore_core_release(SingleProcessDataStore.kt:299)
    at androidx.datastore.SingleProcessDataStore.transformAndWrite(SingleProcessDataStore.kt:282)
    at androidx.datastore.SingleProcessDataStore$actor$1.invokeSuspend(SingleProcessDataStore.kt:165)

Comments

ho...@gmail.com <ho...@gmail.com> #2Nov 16, 2020 01:32AM

I see that with the update to 1.0.0-alpha03, DataStore now throws a more explicit exception indicating that the OutputStream cannot be closed within writeTo().

This makes integrating with Okio's CipherSink much more challenging, as it invokes its Cipher's doFinal() inside of close(). Any chance this requirement could be lifted? If not, feel free to close this issue.

ro...@google.com <ro...@google.com> #3Nov 16, 2020 11:59PM

Marked as fixed.

Thanks for the feedback. I've changed it so that closing the provided OutputStream is a no-op.

https://android-review.googlesource.com/c/platform/frameworks/support/+/1501231

This will be included in alpha5.

ap...@google.com <ap...@google.com> #4Nov 17, 2020 01:50AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c50b27fbbbf252ea5f635ccd86809ca1503a35b1
Author: rohitsat13 <rohitsat@google.com>
Date: Mon Nov 16 14:45:46 2020

Make the Serializer's OutputStream.close() a no-op.

Instead of throwing IllegalStateException when the OutputStream is closed, let's make it a no-op. We need the underlying FileOutputStream to be open after the Serializer has completedso we can get access to and sync the file descriptor. Previously we required that the OutputStream was not closed, but this can be problematic, for example:

b/173037611

Bug: 173037611
Relnote: We now allow (but don't require) the OutputStream passed to Serializer.writeTo() to be closed.
Test: Updated the original output stream closing test.
Change-Id: I5c9bffd60bd12179a496e4589b010e31d00d8a5a

M datastore/datastore-core/src/main/java/androidx/datastore/core/Serializer.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/SingleProcessDataStore.kt
M datastore/datastore-core/src/test/java/androidx/datastore/core/SingleProcessDataStoreTest.kt

https://android-review.googlesource.com/1501231

Issue 173037611

Description

Issue summary

Comments

ho...@gmail.com <ho...@gmail.com> #2Nov 16, 2020 01:32AM

ro...@google.com <ro...@google.com> #3Nov 16, 2020 11:59PM

ap...@google.com <ap...@google.com> #4Nov 17, 2020 01:50AM

Add comment

Issue metadata