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

Unverified Commit be6c3311 authored by Ricki Hirner's avatar Ricki Hirner Committed by GitHub
Browse files

WebDAV: remove notifications, timeout logic (#1630) 

* Refactor openDocument operation to use OsConstants for mode parsing

* RandomAccessCallbackWrapper: refactor so that it's only purpose is to avoid the memory leak

* Use main looper instead of a new thread per RandomAccessCallback

* Remove WebDAV access notification

* Remove nsk90-kstatemachine dependency

* Simplify fileDescriptor() method

* Use dedicated I/O thread again; use Kotlin `copyTo` for copying
parent 75586377

app/build.gradle.kts

+0 −1
Original line number Diff line number Diff line
@@ -188,7 +188,6 @@ dependencies { 
    implementation(libs.dnsjava)

    implementation(libs.guava)

    implementation(libs.mikepenz.aboutLibraries)

    implementation(libs.nsk90.kstatemachine)

    implementation(libs.okhttp.base)

    implementation(libs.okhttp.brotli)

    implementation(libs.okhttp.logging)

app/src/main/kotlin/at/bitfire/davdroid/ui/NotificationRegistry.kt

+0 −1
Original line number Diff line number Diff line
@@ -47,7 +47,6 @@ class NotificationRegistry @Inject constructor( 
        const val NOTIFY_DATABASE_CORRUPTED = 4

        const val NOTIFY_SYNC_ERROR = 10

        const val NOTIFY_INVALID_RESOURCE = 11

        const val NOTIFY_WEBDAV_ACCESS = 12

        const val NOTIFY_SYNC_EXPEDITED = 14

        const val NOTIFY_TASKS_PROVIDER_TOO_OLD = 20

        const val NOTIFY_PERMISSIONS = 21

app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt

+35 −34
Original line number Diff line number Diff line
@@ -5,20 +5,20 @@ 
package at.bitfire.davdroid.webdav



import android.content.Context

import android.os.Handler

import android.os.HandlerThread

import android.os.ParcelFileDescriptor

import android.os.ProxyFileDescriptorCallback

import android.os.storage.StorageManager

import android.system.ErrnoException

import android.system.OsConstants

import android.text.format.Formatter

import androidx.annotation.RequiresApi

import androidx.core.app.NotificationCompat

import androidx.core.app.NotificationManagerCompat

import androidx.core.content.getSystemService

import at.bitfire.dav4jvm.DavResource

import at.bitfire.dav4jvm.HttpUtils

import at.bitfire.dav4jvm.exception.DavException

import at.bitfire.dav4jvm.exception.HttpException

import at.bitfire.davdroid.R

import at.bitfire.davdroid.network.HttpClient

import at.bitfire.davdroid.ui.NotificationRegistry

import at.bitfire.davdroid.util.DavUtils

import com.google.common.cache.CacheBuilder

import com.google.common.cache.CacheLoader
@@ -40,17 +40,17 @@ import okhttp3.MediaType 
import java.io.InterruptedIOException

import java.net.HttpURLConnection

import java.util.logging.Logger

import javax.annotation.WillClose



@RequiresApi(26)

class RandomAccessCallback @AssistedInject constructor(

    @Assisted val httpClient: HttpClient,

    @Assisted val url: HttpUrl,

    @Assisted val mimeType: MediaType?,

    @Assisted @WillClose private val httpClient: HttpClient,

    @Assisted private val url: HttpUrl,

    @Assisted private val mimeType: MediaType?,

    @Assisted headResponse: HeadResponse,

    @Assisted private val externalScope: CoroutineScope,

    @ApplicationContext val context: Context,

    private val logger: Logger,

    private val notificationRegistry: NotificationRegistry

    @ApplicationContext private val context: Context,

    private val logger: Logger

): ProxyFileDescriptorCallback() {



    companion object {
@@ -77,26 +77,34 @@ class RandomAccessCallback @AssistedInject constructor( 
    private val fileSize = headResponse.size ?: throw IllegalArgumentException("Can only be used with given file size")

    private val documentState = headResponse.toDocumentState() ?: throw IllegalArgumentException("Can only be used with ETag/Last-Modified")



    private val notificationManager = NotificationManagerCompat.from(context)

    private val notification = NotificationCompat.Builder(context, notificationRegistry.CHANNEL_STATUS)

        .setPriority(NotificationCompat.PRIORITY_LOW)

        .setCategory(NotificationCompat.CATEGORY_STATUS)

        .setContentTitle(context.getString(R.string.webdav_notification_access))

        .setContentText(dav.fileName())

        .setSubText(Formatter.formatFileSize(context, fileSize))

        .setSmallIcon(R.drawable.ic_storage_notify)

        .setOngoing(true)

    private val notificationTag = url.toString()



    private val pageLoader = PageLoader(externalScope)

    private val pageCache: LoadingCache<PageIdentifier, ByteArray> = CacheBuilder.newBuilder()

        .maximumSize(10)    // don't cache more than 10 entries (MAX_PAGE_SIZE each)

        .softValues()       // use SoftReference for the page contents so they will be garbage collected if memory is needed

        .softValues()       // use SoftReference for the page contents so they will be garbage-collected if memory is needed

        .build(pageLoader)  // fetch actual content using pageLoader



    /** This thread will be used for I/O operations like [onRead]. Using the main looper would cause ANRs. */

    private val ioThread = HandlerThread("WebDAV I/O").apply {

        start()

    }



    private val pagingReader = PagingReader(fileSize, MAX_PAGE_SIZE, pageCache)





    // file descriptor



    /**

     * Returns a random-access file descriptor that can be used in a DocumentsProvider.

     */

    fun fileDescriptor(): ParcelFileDescriptor {

        val storageManager = context.getSystemService<StorageManager>()!!

        val ioHandler = Handler(ioThread.looper)

        return storageManager.openProxyFileDescriptor(ParcelFileDescriptor.MODE_READ_ONLY, this, ioHandler)

    }





    // implementation



    override fun onFsync() { /* not used */ }



    override fun onGetSize(): Long = runBlockingFd("onGetFileSize") {
@@ -117,7 +125,10 @@ class RandomAccessCallback @AssistedInject constructor( 


    override fun onRelease() {

        logger.fine("onRelease")

        notificationManager.cancel(notificationTag, NotificationRegistry.NOTIFY_WEBDAV_ACCESS)



        // free resources

        ioThread.quitSafely()

        httpClient.close()

    }
@@ -185,16 +196,6 @@ class RandomAccessCallback @AssistedInject constructor( 
            val size = key.size

            logger.fine("Loading page $url $offset/$size")



            // update notification

            notificationRegistry.notifyIfPossible(NotificationRegistry.NOTIFY_WEBDAV_ACCESS, tag = notificationTag) {

                val progress =

                    if (fileSize == 0L)     // avoid division by zero

                        100

                    else

                        (offset * 100 / fileSize).toInt()

                notification.setProgress(100, progress, false).build()

            }



            val ifMatch: Headers =

                documentState.eTag?.let { eTag ->

                    Headers.headersOf("If-Match", "\"$eTag\"")

app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallbackWrapper.kt

+36 −133
Original line number Diff line number Diff line
@@ -4,182 +4,85 @@ 


package at.bitfire.davdroid.webdav



import android.os.Handler

import android.os.HandlerThread

import android.os.ProxyFileDescriptorCallback

import android.system.ErrnoException

import android.system.OsConstants

import androidx.annotation.RequiresApi

import at.bitfire.davdroid.network.HttpClient

import at.bitfire.davdroid.webdav.RandomAccessCallbackWrapper.Companion.TIMEOUT_INTERVAL

import dagger.assisted.Assisted

import dagger.assisted.AssistedFactory

import dagger.assisted.AssistedInject

import kotlinx.coroutines.CoroutineScope

import okhttp3.HttpUrl

import okhttp3.MediaType

import ru.nsk.kstatemachine.event.Event

import ru.nsk.kstatemachine.state.State

import ru.nsk.kstatemachine.state.finalState

import ru.nsk.kstatemachine.state.initialState

import ru.nsk.kstatemachine.state.onEntry

import ru.nsk.kstatemachine.state.onExit

import ru.nsk.kstatemachine.state.onFinished

import ru.nsk.kstatemachine.state.state

import ru.nsk.kstatemachine.state.transitionOn

import ru.nsk.kstatemachine.statemachine.StateMachine

import ru.nsk.kstatemachine.statemachine.createStdLibStateMachine

import ru.nsk.kstatemachine.statemachine.processEventBlocking

import java.util.Timer

import java.util.TimerTask

import java.util.logging.Logger

import kotlin.concurrent.schedule



/**

 * (2021/12/02) Currently Android's `StorageManager.openProxyFileDescriptor` has a memory leak:

 * Use this wrapper to ensure that all memory is released as soon as [onRelease] is called.

 *

 * - (2021/12/02) Currently Android's `StorageManager.openProxyFileDescriptor` has a memory leak:

 * the given callback is registered in `com.android.internal.os.AppFuseMount` (which adds it to

 * a [Map]), but is not unregistered anymore. So it stays in the memory until the whole mount

 * is unloaded. See https://issuetracker.google.com/issues/208788568

 *

 * Use this wrapper to

 *

 * - ensure that all memory is released as soon as [onRelease] is called,

 * - provide timeout functionality: [RandomAccessCallback] will be closed when not

 * is unloaded. See https://issuetracker.google.com/issues/208788568.

 * - (2024/08/24) [Fixed in Android.](https://android.googlesource.com/platform/frameworks/base/+/e7dbf78143ba083af7a8ecadd839a9dbf6f01655%5E%21/#F0)

 *

 * used for more than [TIMEOUT_INTERVAL] ms and re-created when necessary.

 * **All fields of objects of this class must be set to `null` when [onRelease] is called!**

 * Otherwise they will leak memory.

 *

 * @param httpClient    HTTP client – [RandomAccessCallbackWrapper] is responsible to close it

 * @param httpClient    HTTP client ([RandomAccessCallbackWrapper] is responsible to close it)

 */

@RequiresApi(26)

class RandomAccessCallbackWrapper @AssistedInject constructor(

    @Assisted private val httpClient: HttpClient,

    @Assisted private val url: HttpUrl,

    @Assisted private val mimeType: MediaType?,

    @Assisted private val headResponse: HeadResponse,

    @Assisted private val externalScope: CoroutineScope,

    private val logger: Logger,

    private val callbackFactory: RandomAccessCallback.Factory

    @Assisted httpClient: HttpClient,

    @Assisted url: HttpUrl,

    @Assisted mimeType: MediaType?,

    @Assisted headResponse: HeadResponse,

    @Assisted externalScope: CoroutineScope,

    callbackFactory: RandomAccessCallback.Factory

): ProxyFileDescriptorCallback() {



    companion object {

        const val TIMEOUT_INTERVAL = 15000L

    }



    @AssistedFactory

    interface Factory {

        fun create(httpClient: HttpClient, url: HttpUrl, mimeType: MediaType?, headResponse: HeadResponse, externalScope: CoroutineScope): RandomAccessCallbackWrapper

    }



    sealed class Events {

        object Transfer : Event

        object NowIdle : Event

        object GoStandby : Event

        object Close : Event

    }

    /* We don't use a sealed class for states here because the states would then be singletons, while we can have

    multiple instances of the state machine (which require multiple instances of the states, too). */

    private val machine = createStdLibStateMachine {

        lateinit var activeIdleState: State

        lateinit var activeTransferringState: State

        lateinit var standbyState: State

        lateinit var closedState: State



        initialState("active") {

            onEntry {

                _callback = callbackFactory.create(httpClient, url, mimeType, headResponse, externalScope)

            }

            onExit {

                _callback?.onRelease()

                _callback = null

            }



            transitionOn<Events.GoStandby> { targetState = { standbyState } }

            transitionOn<Events.Close> { targetState = { closedState } }



            // active has two nested states: transferring (I/O running) and idle (starts timeout timer)

            activeIdleState = initialState("idle") {

                val timer: Timer = Timer(true)

                var timeout: TimerTask? = null

    // callback reference



                onEntry {

                    timeout = timer.schedule(TIMEOUT_INTERVAL) {

                        machine.processEventBlocking(Events.GoStandby)

                    }

                }

                onExit {

                    timeout?.cancel()

                    timeout = null

                }

                onFinished {

                    timer.cancel()

                }



                transitionOn<Events.Transfer> { targetState = { activeTransferringState } }

            }



            activeTransferringState = state("transferring") {

                transitionOn<Events.NowIdle> { targetState = { activeIdleState } }

            }

        }



        standbyState = state("standby") {

            transitionOn<Events.Transfer> { targetState = { activeTransferringState } }

            transitionOn<Events.NowIdle> { targetState = { activeIdleState } }

            transitionOn<Events.Close> { targetState = { closedState } }

        }



        closedState = finalState("closed")

        onFinished {

            shutdown()

        }



        logger = StateMachine.Logger { message ->

            this@RandomAccessCallbackWrapper.logger.finer(message())

        }

    }



    private val workerThread = HandlerThread(javaClass.simpleName).apply { start() }

    val workerHandler: Handler = Handler(workerThread.looper)



    private var _callback: RandomAccessCallback? = null

    /**

     * This field is initialized with a strong reference to the callback. It is cleared when

     * [onRelease] is called so that the garbage collector can remove the actual [RandomAccessCallback].

     */

    private var callbackRef: RandomAccessCallback? =

        callbackFactory.create(httpClient, url, mimeType, headResponse, externalScope)



    fun<T> requireCallback(block: (callback: RandomAccessCallback) -> T): T {

        machine.processEventBlocking(Events.Transfer)

        try {

            return block(_callback ?: throw IllegalStateException())

        } finally {

            machine.processEventBlocking(Events.NowIdle)

        }

    }

    private fun requireCallback(functionName: String): RandomAccessCallback =

        callbackRef ?: throw ErrnoException(functionName, OsConstants.EBADF)





    /// states ///

    // non-interface delegates



    @Synchronized

    private fun shutdown() {

        httpClient.close()

        workerThread.quit()

    }

    fun fileDescriptor() =

        requireCallback("fileDescriptor").fileDescriptor()





    /// delegating implementation of ProxyFileDescriptorCallback ///

    // delegating implementation of ProxyFileDescriptorCallback



    @Synchronized

    override fun onFsync() { /* not used */ }



    @Synchronized

    override fun onGetSize() =

        requireCallback { it.onGetSize() }

        requireCallback("onGetSize").onGetSize()



    @Synchronized

    override fun onRead(offset: Long, size: Int, data: ByteArray) =

        requireCallback { it.onRead(offset, size, data) }

        requireCallback("onRead").onRead(offset, size, data)



    @Synchronized

    override fun onWrite(offset: Long, size: Int, data: ByteArray) =

        requireCallback { it.onWrite(offset, size, data) }

        requireCallback("onWrite").onWrite(offset, size, data)



    @Synchronized

    override fun onRelease() {

        machine.processEventBlocking(Events.Close)

        requireCallback("onRelease").onRelease()



        // remove reference to allow garbage collection

        callbackRef = null

    }



}
 
 No newline at end of file

app/src/main/kotlin/at/bitfire/davdroid/webdav/StreamingFileDescriptor.kt

+32 −97
Original line number Diff line number Diff line
@@ -4,23 +4,15 @@ 


package at.bitfire.davdroid.webdav



import android.content.Context

import android.os.ParcelFileDescriptor

import android.text.format.Formatter

import androidx.annotation.WorkerThread

import androidx.core.app.NotificationCompat

import androidx.core.app.NotificationManagerCompat

import at.bitfire.dav4jvm.DavResource

import at.bitfire.dav4jvm.exception.HttpException

import at.bitfire.davdroid.R

import at.bitfire.davdroid.di.IoDispatcher

import at.bitfire.davdroid.network.HttpClient

import at.bitfire.davdroid.ui.NotificationRegistry

import at.bitfire.davdroid.util.DavUtils

import dagger.assisted.Assisted

import dagger.assisted.AssistedFactory

import dagger.assisted.AssistedInject

import dagger.hilt.android.qualifiers.ApplicationContext

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.launch
@@ -32,27 +24,21 @@ import okio.BufferedSink 
import java.io.IOException

import java.util.logging.Level

import java.util.logging.Logger

import javax.annotation.WillClose



/**

 * @param client    HTTP client– [StreamingFileDescriptor] is responsible to close it

 * @param client    HTTP client ([StreamingFileDescriptor] is responsible to close it)

 */

class StreamingFileDescriptor @AssistedInject constructor(

    @Assisted private val client: HttpClient,

    @Assisted @WillClose private val client: HttpClient,

    @Assisted private val url: HttpUrl,

    @Assisted private val mimeType: MediaType?,

    @Assisted private val externalScope: CoroutineScope,

    @Assisted private val finishedCallback: OnSuccessCallback,

    @ApplicationContext private val context: Context,

    @IoDispatcher private val ioDispatcher: CoroutineDispatcher,

    private val logger: Logger,

    private val notificationRegistry: NotificationRegistry

    private val logger: Logger

) {



    companion object {

        /** 1 MB transfer buffer */

        private const val BUFFER_SIZE = 1024*1024

    }



    @AssistedFactory

    interface Factory {

        fun create(client: HttpClient, url: HttpUrl, mimeType: MediaType?, externalScope: CoroutineScope, finishedCallback: OnSuccessCallback): StreamingFileDescriptor
@@ -61,28 +47,21 @@ class StreamingFileDescriptor @AssistedInject constructor( 
    val dav = DavResource(client.okHttpClient, url)

    var transferred: Long = 0



    private val notificationManager = NotificationManagerCompat.from(context)

    private val notification = NotificationCompat.Builder(context, notificationRegistry.CHANNEL_STATUS)

        .setPriority(NotificationCompat.PRIORITY_LOW)

        .setCategory(NotificationCompat.CATEGORY_STATUS)

        .setContentText(dav.fileName())

        .setSmallIcon(R.drawable.ic_storage_notify)

        .setOngoing(true)

    val notificationTag = url.toString()





    fun download() = doStreaming(false)

    fun upload() = doStreaming(true)



    private fun doStreaming(upload: Boolean): ParcelFileDescriptor {

        val (readFd, writeFd) = ParcelFileDescriptor.createReliablePipe()



        externalScope.launch(ioDispatcher) {

        var success = false

        externalScope.launch {

            try {

                if (upload)

                    uploadNow(readFd)

                else

                    downloadNow(writeFd)



                success = true

            } catch (e: HttpException) {

                logger.log(Level.WARNING, "HTTP error when opening remote file", e)

                writeFd.closeWithError("${e.code} ${e.message}")
@@ -90,17 +69,15 @@ class StreamingFileDescriptor @AssistedInject constructor( 
                logger.log(Level.INFO, "Couldn't serve file (not necessarily an error)", e)

                writeFd.closeWithError(e.message)

            } finally {

                client.close()

            }



                // close pipe

                try {

                    readFd.close()

                    writeFd.close()

                } catch (_: IOException) {}



            notificationManager.cancel(notificationTag, NotificationRegistry.NOTIFY_WEBDAV_ACCESS)



            finishedCallback.onSuccess(transferred)

                client.close()

                finishedCallback.onFinished(transferred, success)

            }

        }



        return if (upload)
@@ -109,50 +86,21 @@ class StreamingFileDescriptor @AssistedInject constructor( 
            readFd

    }



    @WorkerThread

    private suspend fun downloadNow(writeFd: ParcelFileDescriptor) = runInterruptible {

    /**

     * Downloads a WebDAV resource.

     *

     * @param writeFd   destination file descriptor (could for instance represent a local file)

     */

    private suspend fun downloadNow(writeFd: ParcelFileDescriptor) = runInterruptible(ioDispatcher) {

        dav.get(DavUtils.acceptAnything(preferred = mimeType), null) { response ->

            response.body.use { body ->

                if (response.isSuccessful) {

                    val length = body.contentLength()



                    notification.setContentTitle(context.getString(R.string.webdav_notification_download))

                    if (length == -1L)

                        // unknown file size, show notification now (no updates on progress)

                        notificationRegistry.notifyIfPossible(NotificationRegistry.NOTIFY_WEBDAV_ACCESS, notificationTag) {

                            notification

                                .setProgress(100, 0, true)

                                .build()

                        }

                    else

                        // known file size

                        notification.setSubText(Formatter.formatFileSize(context, length))



                    ParcelFileDescriptor.AutoCloseOutputStream(writeFd).use { output ->

                        val buffer = ByteArray(BUFFER_SIZE)

                    ParcelFileDescriptor.AutoCloseOutputStream(writeFd).use { destination ->

                        body.byteStream().use { source ->

                            // read first chunk

                            var bytes = source.read(buffer)

                            while (bytes != -1) {

                                // update notification (if file size is known)

                                if (length > 0)

                                    notificationRegistry.notifyIfPossible(NotificationRegistry.NOTIFY_WEBDAV_ACCESS, notificationTag) {

                                        val progress = (transferred*100/length).toInt()

                                        notification

                                            .setProgress(100, progress, false)

                                            .build()

                                    }



                                // write chunk

                                output.write(buffer, 0, bytes)

                                transferred += bytes



                                // read next chunk

                                bytes = source.read(buffer)

                            transferred += source.copyTo(destination)

                        }

                        logger.finer("Downloaded $transferred byte(s) from $url")

                    }

                    }



                } else

                    writeFd.closeWithError("${response.code} ${response.message}")
@@ -160,31 +108,18 @@ class StreamingFileDescriptor @AssistedInject constructor( 
        }

    }



    @WorkerThread

    private suspend fun uploadNow(readFd: ParcelFileDescriptor) = runInterruptible {

    /**

     * Uploads a WebDAV resource.

     *

     * @param readFd    source file descriptor (could for instance represent a local file)

     */

    private suspend fun uploadNow(readFd: ParcelFileDescriptor) = runInterruptible(ioDispatcher) {

        val body = object: RequestBody() {

            override fun contentType(): MediaType? = mimeType

            override fun isOneShot() = true

            override fun writeTo(sink: BufferedSink) {

                notificationRegistry.notifyIfPossible(NotificationRegistry.NOTIFY_WEBDAV_ACCESS, notificationTag) {

                    notification

                        .setContentTitle(context.getString(R.string.webdav_notification_upload))

                        .build()

                }



                ParcelFileDescriptor.AutoCloseInputStream(readFd).use { input ->

                    val buffer = ByteArray(BUFFER_SIZE)



                    // read first chunk

                    var size = input.read(buffer)

                    while (size != -1) {

                        // write chunk

                        sink.write(buffer, 0, size)

                        transferred += size



                        // read next chunk

                        size = input.read(buffer)

                    }

                    transferred += input.copyTo(sink.outputStream())

                    logger.finer("Uploaded $transferred byte(s) to $url")

                }

            }
@@ -196,7 +131,7 @@ class StreamingFileDescriptor @AssistedInject constructor( 




    fun interface OnSuccessCallback {

        fun onSuccess(transferred: Long)

        fun onFinished(transferred: Long, success: Boolean)

    }



}
 
 No newline at end of file