Skip to content
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.

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

Add visibility of proguard rules to README or wiki please #3097

Closed
isuPatches opened this issue Jul 21, 2015 · 28 comments
Closed

Add visibility of proguard rules to README or wiki please #3097

isuPatches opened this issue Jul 21, 2015 · 28 comments
Labels

Comments

@isuPatches
Copy link

Are there examples of proguard rules that will work with minifyEnabled set to true? I'm currently getting:

 java.lang.NoSuchMethodError: No virtual method doOnSubscribe(Lrx/functions/Action0;)Lrx/Observable
@ZacSweers
Copy link
Contributor

+1

The unsafe package does some reflective lookups of fields that get lost in obfuscation, and the resulting NoSuchFieldException gets thrown up.

@julienbanse
Copy link

+1
Actually, I have NoSuchFieldException: producerIndex with the release 1.0.14 (not happens in 1.0.13)

My current proguard settings for RxJava is

-keep class rx.schedulers.Schedulers {
    public static <methods>;
}
-keep class rx.schedulers.ImmediateScheduler {
    public <methods>;
}
-keep class rx.schedulers.TestScheduler {
    public <methods>;
}
-keep class rx.schedulers.Schedulers {
    public static ** test();
}

@ZacSweers
Copy link
Contributor

I'm able to avoid it by keeping everything in rx.internal.util.unsafe, but would be nice to avoid the reflection entirely if possible.

@julienbanse
Copy link

Yes, this is what I did to patch this issue.

@gokhanbarisaker
Copy link

Full stack trace for NoSuchFieldException, that @julienbanse mentioned.

08-14 14:58:28.619    7362-7429/com.markavip.app E/AndroidRuntime﹕ FATAL EXCEPTION: pool-1-thread-1
    Process: com.markavip.app, PID: 7362
    java.lang.IllegalStateException: Fatal Exception thrown on Scheduler.Worker thread.
            at rx.internal.schedulers.ScheduledAction.run(SourceFile:62)
            at rx.schedulers.ExecutorScheduler$ExecutorSchedulerWorker.run(SourceFile:98)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
            at java.lang.Thread.run(Thread.java:818)
     Caused by: java.lang.InternalError
            at rx.internal.util.unsafe.UnsafeAccess.a(SourceFile:103)
            at rx.internal.util.unsafe.SpscArrayQueueProducerFields.<clinit>(SourceFile:39)
            at rx.internal.operators.OperatorObserveOn$ObserveOnSubscriber.<init>(SourceFile:102)
            at rx.internal.operators.OperatorObserveOn.a(SourceFile:64)
            at rx.internal.operators.OperatorObserveOn.a(SourceFile:44)
            at rx.Observable$2.a(SourceFile:158)
            at rx.Observable$2.a(SourceFile:154)
            at rx.Observable.a(SourceFile:7710)
            at rx.internal.operators.OperatorSubscribeOn$1$1.a(SourceFile:62)
            at rx.internal.schedulers.ScheduledAction.run(SourceFile:55)
            at rx.schedulers.ExecutorScheduler$ExecutorSchedulerWorker.run(SourceFile:98)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
            at java.lang.Thread.run(Thread.java:818)
     Caused by: java.lang.NoSuchFieldException: producerIndex
            at java.lang.Class.getDeclaredField(Class.java:890)
            at rx.internal.util.unsafe.UnsafeAccess.a(SourceFile:100)
            at rx.internal.util.unsafe.SpscArrayQueueProducerFields.<clinit>(SourceFile:39)
            at rx.internal.operators.OperatorObserveOn$ObserveOnSubscriber.<init>(SourceFile:102)
            at rx.internal.operators.OperatorObserveOn.a(SourceFile:64)
            at rx.internal.operators.OperatorObserveOn.a(SourceFile:44)
            at rx.Observable$2.a(SourceFile:158)
            at rx.Observable$2.a(SourceFile:154)
            at rx.Observable.a(SourceFile:7710)
            at rx.internal.operators.OperatorSubscribeOn$1$1.a(SourceFile:62)
            at rx.internal.schedulers.ScheduledAction.run(SourceFile:55)
            at rx.schedulers.ExecutorScheduler$ExecutorSchedulerWorker.run(SourceFile:98)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
            at java.lang.Thread.run(Thread.java:818)

ZacSweers referenced this issue in JakeWharton/RxBinding Aug 25, 2015
Since it's accessed via reflection, this needs to be kept un-obfuscated.
@desseim
Copy link

desseim commented Aug 26, 2015

Thing is, this used to JustWork(tm) because the reflective call was done directly on the target class (e.g. SpscArrayQueueProducerFields.class.getDeclaredField("producerIndex")) and proguard could figure out that this field needed to be kept (see the "reflection" chapter of the proguard documentation).
Starting with 1.0.14 the reflective call is now indirected through a utility class (UnsafeAccess#addressOf) and I guess proguard don't detect the field to keep anymore.

For now, we can either keep everything in package unsafe or selectively keep reflected field with

-keepclassmembers class rx.internal.util.unsafe.*ArrayQueue*Field* {
    long producerIndex;
    long consumerIndex;
}
-keepclassmembers class rx.internal.util.unsafe.BaseLinkedQueueProducerNodeRef {
    long producerNode;
    long consumerNode;
}

while waiting for them to be annotated appropriately in the source.

@JakeWharton
Copy link
Member

FWIW for Android at least this is going to be handled by ReactiveX/RxAndroid#219 automatically for consumers. This obviously does not solve if you are ProGuarding for another target (or simply not using RxAndroid on Android).

@arturonaredo
Copy link

@julienbanse + @desseim responses together worked for me.

@jaredsburrows
Copy link

@desseim Adding those rules produces these "noted" warnings but it works!:

Note: the configuration refers to the unknown field 'long producerNode' in class 'rx.internal.util.unsafe.BaseLinkedQueueProducerNodeRef'
Note: the configuration refers to the unknown field 'long consumerNode' in class 'rx.internal.util.unsafe.BaseLinkedQueueProducerNodeRef'

@desseim
Copy link

desseim commented Nov 20, 2015

@jaredsburrows You're right, thanks for noticing.
Seems like a dumb copy-paste error... Anyway, the proper settings should be:

-keepclassmembers class rx.internal.util.unsafe.*ArrayQueue*Field* {
    long producerIndex;
    long consumerIndex;
}
-keepclassmembers class rx.internal.util.unsafe.BaseLinkedQueueProducerNodeRef {
    rx.internal.util.atomic.LinkedQueueNode producerNode;
}
-keepclassmembers class rx.internal.util.unsafe.BaseLinkedQueueConsumerNodeRef {
    rx.internal.util.atomic.LinkedQueueNode consumerNode;
}

On my code base (and yours too apparently), BaseLinkedQueueProducerNodeRef and BaseLinkedQueueConsumerNodeRef classes altogether were stripped away by Proguard anyway so that no problem arose.
For anyone whose code ended up BaseLinkedQueue though, it would have probably thrown at runtime.

felipecsl added a commit to felipecsl/RxAndroid that referenced this issue Nov 20, 2015
@jaredsburrows
Copy link

@desseim @felipesci Thank you so much for posting this.

I have recently updated rxandroid and rxjava:

    compile 'io.reactivex:rxandroid:1.1.0'
    compile 'io.reactivex:rxjava:1.1.0'

and those current proguard rules do not seem to work anymore.

@artem-zinnatullin
Copy link
Contributor

@jaredsburrows I've backed ProGuard rules for RxJava as aar and we successfully use it in production for several months (updated each RxJava release), can you please try it? https://github.com/artem-zinnatullin/RxJavaProGuardRules

// if you'll face a problem — feel free to create an issue in the repository of the rules and I'll make a fix and ship new version.

@jaredsburrows
Copy link

@artem-zinnatullin Sure! I believe I am using the same rules as your dependency is providing. You have tested this against 1.1.0?

@artem-zinnatullin
Copy link
Contributor

@jaredsburrows yep, we use it with 1.1.0, can you please post your problem (stacktraces/etc) as an issue to the repository? Or here as a comment, though I'm not sure that it's the right place.

@jaredsburrows
Copy link

@artem-zinnatullin I am using this with Retrofit and I see the logs with proguard on but I am not seeing the UI get updated. I am still investigating and I will try to keep you updated.

@jaredsburrows
Copy link

@artem-zinnatullin Just a quick update. The issue I was seeing was related to another issue with a few proguard rules and not related to the rules you have provided. Your rules work very well. Thanks for your support.

@artem-zinnatullin
Copy link
Contributor

Ok, thanks for update 👍

@akarnokd
Copy link
Member

akarnokd commented Apr 2, 2016

I'm closing this issue due to inactivity. If you have further input on the issue, don't hesitate to reopen this issue or post a new one.

@akarnokd akarnokd closed this as completed Apr 2, 2016
@rzsombor
Copy link
Contributor

rzsombor commented Jun 12, 2016

We're using RxJava 1.1.5 in production currently for Android with ProGuard, and according to crash logs this issue pops up time to time on certain device + os conditions. To be specific the conditions seem to be (out of literally millions of sessions):

  • Only on Android 5.0, 5.0.1 or 5.0.2
  • Only on Samsung devices (sm_g900f, sm_n9005, gt_i9505 and some unknown, but still supposedly Samsung ones)
  • ProGuard is on, but obfuscation is off and the relevant ProGuard config is as follows:

-keep class rx.* { ; }
-dontwarn rx.internal.

The crash rate is really low, but it's certainly happening. Has anyone encountered with this? There seem to be some funky vendor-specific thing going on there. So far we couldn't test this in house with the problematic devices.

Edit:
Apparently Samsung doesn't really give much support on this: http://developer.samsung.com/forum/board/thread/view.do?boardName=General&messageId=286452

@artem-zinnatullin
Copy link
Contributor

See #3459

@JakeWharton
Copy link
Member

With those rules there's no point in using ProGuard. You're keeping the
entire library and preventing ProGuard from actually doing anything.

On Sun, Jun 12, 2016, 5:04 AM Artem Zinnatullin notifications@github.com
wrote:

See #3459 #3459


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#3097 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAEEEQwhWw6x7B83eDHcB4WB14Y4J4oTks5qK8uUgaJpZM4FdG5S
.

@jaredsburrows
Copy link

Just to clear this up, again, please see @desseim's post here: #3097 (comment).

@artem-zinnatullin
Copy link
Contributor

Probably they want to shrink all other libraries and keep RxJava to avoid
crash on Samsung ¯_(ツ)_/¯

On Sun, 12 Jun 2016, 18:05 Jake Wharton, notifications@github.com wrote:

With those rules there's no point in using ProGuard. You're keeping the
entire library and preventing ProGuard from actually doing anything.

On Sun, Jun 12, 2016, 5:04 AM Artem Zinnatullin notifications@github.com
wrote:

See #3459 #3459


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3097 (comment)
,
or mute the thread
<
https://github.com/notifications/unsubscribe/AAEEEQwhWw6x7B83eDHcB4WB14Y4J4oTks5qK8uUgaJpZM4FdG5S

.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3097 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA7B3MhxRgW-Cru7yayoFi2xaLfvwgrvks5qLCBGgaJpZM4FdG5S
.

@rzsombor
Copy link
Contributor

rzsombor commented Jun 13, 2016

Well yes, this ProGuard config doesn't do a thing with RxJava, we mostly use to to shrink down other stuff. This might not be the most optimal thing to do, but the point here is that we're keeping everything from Rx and the issue still happens.

I might miss something, but the way I see it, the links you provided discuss how ProGuard might screw things up with a config that enables it to shrink RxJava.

My hunch is still that this is due to some funky Samsung-specific thing on their Android versions I mentioned. As a short-term workaround, I'll use the 'rx.unsafe-disable' to disable Unsafe.

@artem-zinnatullin
Copy link
Contributor

You can disable Unsafe only on Samsung devices btw.

On Mon, 13 Jun 2016, 11:00 Zsombor Erdődy-Nagy, notifications@github.com
wrote:

Well yes, this ProGuard config doesn't do a thing with RxJava, we mostly
use to to shrink down other stuff. This might not be the most optimal thing
to do, but the point here is that we're keeping everything from Rx and the
issue still happens.

I might miss something, but the way I see it, the links you provided
discuss how ProGuard might screw things up with a config that enables it to
shrink RxJava. As a short-term workaround, I'll use the 'rx.unsafe-disable'
to disable Unsafe.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3097 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA7B3Ai3ZgaMc1zjg9vFSavLX_1hVh6Lks5qLQ42gaJpZM4FdG5S
.

@eranpolo
Copy link

@rzsombor, we encountered a very similar issue with a different library (greenDAO), maybe it could shed some light on the issue you encountered here.

We get:
System.err: Caused by: java.lang.NoSuchFieldException: TABLENAME
System.err: at java.lang.Class.getField(Class.java:1104)
System.err: at de.greenrobot.dao.internal.DaoConfig.(SourceFile:56)

Only happens on Samsung 5.0/5.0.1/5.0.2 devices in the wild, we can't reproduce this issue in-house.

We have the recommended greenDAO proguard configuration.

Decompiling our APK shows that the "problematic" class does indeed have a public TABLENAME field:
.field public static final TABLENAME:Ljava/lang/String; = "GLIDE_USER"

So we can only join your conclusion that it's a weird Samsung bug. Unfortunately, we don't even have a work-around... Kinda hoping the guys in this project can come up with something...

Btw, other people encountered the same issue with greenDAO (note that the suggested solutions such as adding proguard rules or upgrading to the latest version do not solve the issue):
http://stackoverflow.com/questions/34216163/daoconfig-init-failure-with-greendao-on-samsung-devices-with-android-5-0
greenrobot/greenDAO#188

@rzsombor
Copy link
Contributor

rzsombor commented Jun 13, 2016

@artem-zinnatullin Oh yes, forgot to add that - only switching Unsafe off on these problematic device - OS combinations.

@eranpolo Thanks the extra info. I'm still trying to get my hands on an affected device to get to the bottom of this.

Edit: for reference, our current naive workaround for the Samsung-issue:

if (Build.VERSION.SDK_INT == Build.VERSION_CODES.LOLLIPOP && "samsung".equalsIgnoreCase(Build.MANUFACTURER)) {
    System.setProperty("rx.unsafe-disable", "True");
}

@yishai-glide
Copy link

@rzsombor a list of devices that it happened to here are:

  • SM-N915v
  • SM-S978L
  • SM-N900T
  • SM-G900V
  • SMG900P
  • SAMSUNG-SM-G870A
  • SM-T530NU
  • SM-T350
  • SM-P550
  • GT-I9515

all of those devices had this crash after adding the proguard rules from greenrobot/greenDAO#188

timgreen added a commit to timgreen/opaler that referenced this issue Feb 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests