fix: catch exceptions from closing patches

ReVanced · Jun 14, 2023 · d5d6f85 · d5d6f85
1 parent b8151eb
commit d5d6f85
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 22 deletions.
diff --git a/src/main/kotlin/app/revanced/patcher/Patcher.kt b/src/main/kotlin/app/revanced/patcher/Patcher.kt
@@ -23,6 +23,7 @@ import lanchon.multidexlib2.MultiDexIO
 import org.jf.dexlib2.Opcodes
 import org.jf.dexlib2.iface.DexFile
 import org.jf.dexlib2.writer.io.MemoryDataStore
+import java.io.Closeable
 import java.io.File
 import java.io.OutputStream
 import java.nio.file.Files
@@ -350,24 +351,42 @@ class Patcher(private val options: PatcherOptions) {
 
             val executedPatches = LinkedHashMap<String, ExecutedPatch>() // first is name
 
-            try {
-                context.patches.forEach { patch ->
-                    val patchResult = executePatch(patch, executedPatches)
 
-                    val result = if (patchResult.isSuccess()) {
-                        Result.success(patchResult.success()!!)
-                    } else {
-                        Result.failure(patchResult.error()!!)
-                    }
+            context.patches.forEach { patch ->
+                val patchResult = executePatch(patch, executedPatches)
 
-                    yield(patch.patchName to result)
-                    if (stopOnError && patchResult.isError()) return@sequence
-                }
-            } finally {
-                executedPatches.values.filter { it.success }.reversed().forEach { (patch, _) ->
-                    patch.close()
+                val result = if (patchResult.isSuccess()) {
+                    Result.success(patchResult.success()!!)
+                } else {
+                    Result.failure(patchResult.error()!!)
                 }
+
+                // TODO: This prints before the patch really finishes in case it is a Closeable
+                //  because the Closeable is closed after all patches are executed.
+                yield(patch.patchName to result)
+
+                if (stopOnError && patchResult.isError()) return@sequence
             }
+
+            executedPatches.values
+                .filter(ExecutedPatch::success)
+                .map(ExecutedPatch::patchInstance)
+                .filterIsInstance(Closeable::class.java)
+                .asReversed().forEach {
+                    try {
+                        it.close()
+                    } catch (exception: Exception) {
+                        val patchName = (it as Patch<Context>).javaClass.patchName
+
+                        logger.error("Failed to close '$patchName': ${exception.stackTraceToString()}")
+
+                        yield(patchName to Result.failure(exception))
+
+                        // This is not failsafe. If a patch throws an exception while closing,
+                        // the other patches that depend on it may fail.
+                        if (stopOnError) return@sequence
+                    }
+                }
         }
     }
 

diff --git a/src/main/kotlin/app/revanced/patcher/patch/Patch.kt b/src/main/kotlin/app/revanced/patcher/patch/Patch.kt
@@ -12,21 +12,14 @@ import java.io.Closeable
  * If it implements [Closeable], it will be closed after all patches have been executed.
  * Closing will be done in reverse execution order.
  */
-sealed interface Patch<out T : Context> : Closeable {
+sealed interface Patch<out T : Context> {
     /**
      * The main function of the [Patch] which the patcher will call.
      *
      * @param context The [Context] the patch will work on.
      * @return The result of executing the patch.
      */
     fun execute(context: @UnsafeVariance T): PatchResult
-
-    /**
-     * The closing function for this patch.
-     *
-     * This can be treated like popping the patch from the current patch stack.
-     */
-    override fun close() {}
 }
 
 /**