-
Notifications
You must be signed in to change notification settings - Fork 74
Fix Zlib bug in android 12 and above #193
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Thanks a lot for this. At my organisation, we are currently dealing with the same issue, so your solution is really helpful to us. I have two questions about your solution:
Note that this repo appears to have been abandoned six years ago, so I wouldn't expect this PR to ever get merged. |
Your welcome, Yes of course, the |
Thanks, much appreciated! In case we make any improvements on top of your code, we'll be happy to contribute them back to your repos. Regarding the affected Android versions, So from your answer I gather there's no reason to believe that the fix will create problems on any Android version. We'll test it anyway on various versions from 5 through 13. Will report back here in case we encounter any issues. |
Can you share more information about the cause of the problem?I test it is work right on android 11。I guess maybe google use Zopfli replace zlib impl in android 12。 |
Zlib version has been updated in Android 12 |
I found standalone zlib version patch work right, but the native memory is not free right, do you have the same problem? |
* of the Deflater object is undefined. | ||
*/ | ||
public void end() { | ||
synchronized (zsRef) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should call super.end() to avoid native leak memory!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure? The CustomDeflater
has been cloned from Deflater
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that CustomDeflater
inherits from Deflater
without actually making use of any of its fields, though those fields do get initialised by the parent class's default constructor. Even without memory leaks, this is less efficient than it could be.
Inspired by your solution, we made our own, where we introduced an interface IDeflater
to avoid this issue. We'll publish our solution properly in the next couple of days, but you can already view its sources if you're interested.
Thanks again for your solution. Inspired by it, we created our own, which bundles zlib 1.2.13 binaries (the currently latest version) and creating a version of Sources: https://github.com/EIDU/archive-patcher-android The library is on Maven Central. See the repo's |
We are currently dealing with the same issue. I understood that this change creates custom deflater which uses the bundled zlib. But trying to understand the issue with google archive patcher whether the issue with deflater or zlib. Because in the beginning of the conversation, @hamid97m mentioned issue due to Zlib version upgrade but you mentioned no need to downgrade to any older version. I have below queries related to this change. Please help me answer these.
|
Excellent questions!
I don't actually know why zlib behaves differently on those newer Android versions, only that it does.
No fix was necessary.
It might not actually be considered an "issue", because the output produced by the Android-bundled zlib isn't wrong as far as I'm aware, it's just different (perhaps it's even better in some way, who knows). But even if the behaviour was going to be reverted in Android's zlib, that would not fix our issue for devices that are already out there in the wild and will not receive an Android update anytime soon or ever. |
The archive-patcher can decompressed APK & completely restore the APK file when patch, Can do this relies heavily on the compressed encoding of zlib library: because the encoding generated by different(or optimized) zlib versions is slightly different, the APK file will be corrupted. |
In Android 12 and above, the archive-patcher applier doesn't work because, in Android 12, the Zlib is updated.
In this pull request, I just added a customDeflater class instead Deflater and load the suitable Zlib library version.