Skip to content

fix: Replace dex input streams with file references #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hoo-dles wants to merge 3 commits into from
Closed

fix: Replace dex input streams with file references #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a19c57
replacing dex patch's input stream with file reference
May 15, 2025
899b460
Merge branch 'dev' into fix/hanging-dex-streams
May 15, 2025
0389794
Updating api
May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion api/revanced-patcher.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ public final class app/revanced/patcher/PatcherResult {
}

public final class app/revanced/patcher/PatcherResult$PatchedDexFile {
public final fun getFile ()Ljava/io/File;
public final fun getName ()Ljava/lang/String;
public final fun getStream ()Ljava/io/InputStream;
Copy link
Member

@oSumAtrIX oSumAtrIX May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks API, please add this back with a deprecated annotation

}

public final class app/revanced/patcher/PatcherResult$PatchedResources {
Expand Down Expand Up @@ -171,9 +171,12 @@ public final class app/revanced/patcher/patch/BytecodePatchContext : app/revance
public final class app/revanced/patcher/patch/Option {
public fun <init> (Ljava/lang/String;Ljava/lang/Object;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;ZLkotlin/reflect/KType;Lkotlin/jvm/functions/Function2;)V
public synthetic fun <init> (Ljava/lang/String;Ljava/lang/Object;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;ZLkotlin/reflect/KType;Lkotlin/jvm/functions/Function2;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Ljava/lang/String;Ljava/lang/Object;Ljava/util/Map;Ljava/lang/String;ZLkotlin/reflect/KType;Lkotlin/jvm/functions/Function2;)V
public synthetic fun <init> (Ljava/lang/String;Ljava/lang/Object;Ljava/util/Map;Ljava/lang/String;ZLkotlin/reflect/KType;Lkotlin/jvm/functions/Function2;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
Copy link
Author

@hoo-dles hoo-dles May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure where these changes came from. Maybe previous merge that didn't run apiDump?

public final fun getDefault ()Ljava/lang/Object;
public final fun getDescription ()Ljava/lang/String;
public final fun getKey ()Ljava/lang/String;
public final fun getName ()Ljava/lang/String;
public final fun getRequired ()Z
public final fun getTitle ()Ljava/lang/String;
public final fun getType ()Lkotlin/reflect/KType;
Expand Down
4 changes: 2 additions & 2 deletions src/main/kotlin/app/revanced/patcher/PatcherResult.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ class PatcherResult internal constructor(
* A dex file.
*
* @param name The original name of the dex file.
* @param stream The dex file as [InputStream].
* @param file The dex file as [File].
*/
class PatchedDexFile internal constructor(val name: String, val stream: InputStream)
class PatchedDexFile internal constructor(val name: String, val file: File)

/**
* The resources of a patched apk.
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/app/revanced/patcher/patch/BytecodePatchContext.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class BytecodePatchContext internal constructor(private val config: PatcherConfi
DexIO.DEFAULT_MAX_DEX_POOL_SIZE,
) { _, entryName, _ -> logger.info { "Compiled $entryName" } }
}.listFiles(FileFilter { it.isFile })!!.map {
PatcherResult.PatchedDexFile(it.name, it.inputStream())
Copy link
Member

@oSumAtrIX oSumAtrIX May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input stream can be simply closed here via a use block. No need to convert to a file.

Copy link
Author

@hoo-dles hoo-dles May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just adding a use block here will leave scope and close the stream before PatcherResult.applyTo() is called.

This is why I advocate passing around a file reference instead of an open stream; it's much easier to reason about.

Copy link
Member

@oSumAtrIX oSumAtrIX May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that Patcher is dumping the dex to a file is not something that any other library should take advantage of. It is merely done for caching purposes and no other purpose. In the most basic form patcher has no "requirements" for or does not require anyone to work with files. A stream is all that's needed. If a consuming library needs to convert it to a file it's free to, if one doesn't then there is no reason to require working with files in the first place. If the stream needs to be open, then the use block should not be used here and whatever consumes the stream should simply close the stream.

PatcherResult.PatchedDexFile(it.name, it)
}.toSet()

System.gc()
Expand Down
Loading