IssueTracker

The issue is that Jetifier modifies jars and does not remove signatures. Below is the original class:

Classfile /tmp/non-jetified/org/bouncycastle/jce/provider/ExtCRLException.class
  Last modified Mar 1, 2015; size 362 bytes
  MD5 checksum 3968a716f9e60e1ad36c34157bd0239f
class org.bouncycastle.jce.provider.ExtCRLException extends java.security.cert.CRLException
  minor version: 0
  major version: 49
  flags: ACC_SUPER
Constant pool:
   #1 = Utf8               <init>
   #2 = Utf8               cause
   #3 = Utf8               getCause
   #4 = Utf8               java/security/cert/CRLException
   #5 = Utf8               org/bouncycastle/jce/provider/ExtCRLException
   #6 = Class              #4             // java/security/cert/CRLException
   #7 = Class              #5             // org/bouncycastle/jce/provider/ExtCRLException
   #8 = Utf8               Ljava/lang/Throwable;
   #9 = Utf8               (Ljava/lang/String;)V
  #10 = Utf8               ()Ljava/lang/Throwable;
  #11 = Utf8               (Ljava/lang/String;Ljava/lang/Throwable;)V
  #12 = NameAndType        #2:#8          // cause:Ljava/lang/Throwable;
  #13 = NameAndType        #1:#9          // "<init>":(Ljava/lang/String;)V
  #14 = Fieldref           #7.#12         // org/bouncycastle/jce/provider/ExtCRLException.cause:Ljava/lang/Throwable;
  #15 = Methodref          #6.#13         // java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
  #16 = Utf8               Code
{
  java.lang.Throwable cause;
    descriptor: Ljava/lang/Throwable;
    flags:

  org.bouncycastle.jce.provider.ExtCRLException(java.lang.String, java.lang.Throwable);
    descriptor: (Ljava/lang/String;Ljava/lang/Throwable;)V
    flags:
    Code:
      stack=2, locals=3, args_size=3
         0: aload_0
         1: aload_1
         2: invokespecial #15                 // Method java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
         5: aload_0
         6: aload_2
         7: putfield      #14                 // Field cause:Ljava/lang/Throwable;
        10: return

  public java.lang.Throwable getCause();
    descriptor: ()Ljava/lang/Throwable;
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: getfield      #14                 // Field cause:Ljava/lang/Throwable;
         4: areturn
}

One processed with Jetifier:

Classfile /tmp/jetified/org/bouncycastle/jce/provider/ExtCRLException.class
  Last modified Oct 14, 2019; size 362 bytes
  MD5 checksum 76bc557706ee3480e53d88b21316913f
class org.bouncycastle.jce.provider.ExtCRLException extends java.security.cert.CRLException
  minor version: 0
  major version: 49
  flags: ACC_SUPER
Constant pool:
   #1 = Utf8               org/bouncycastle/jce/provider/ExtCRLException
   #2 = Class              #1             // org/bouncycastle/jce/provider/ExtCRLException
   #3 = Utf8               java/security/cert/CRLException
   #4 = Class              #3             // java/security/cert/CRLException
   #5 = Utf8               cause
   #6 = Utf8               Ljava/lang/Throwable;
   #7 = Utf8               <init>
   #8 = Utf8               (Ljava/lang/String;Ljava/lang/Throwable;)V
   #9 = Utf8               (Ljava/lang/String;)V
  #10 = NameAndType        #7:#9          // "<init>":(Ljava/lang/String;)V
  #11 = Methodref          #4.#10         // java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
  #12 = NameAndType        #5:#6          // cause:Ljava/lang/Throwable;
  #13 = Fieldref           #2.#12         // org/bouncycastle/jce/provider/ExtCRLException.cause:Ljava/lang/Throwable;
  #14 = Utf8               getCause
  #15 = Utf8               ()Ljava/lang/Throwable;
  #16 = Utf8               Code
{
  java.lang.Throwable cause;
    descriptor: Ljava/lang/Throwable;
    flags:

  org.bouncycastle.jce.provider.ExtCRLException(java.lang.String, java.lang.Throwable);
    descriptor: (Ljava/lang/String;Ljava/lang/Throwable;)V
    flags:
    Code:
      stack=2, locals=3, args_size=3
         0: aload_0
         1: aload_1
         2: invokespecial #11                 // Method java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
         5: aload_0
         6: aload_2
         7: putfield      #13                 // Field cause:Ljava/lang/Throwable;
        10: return

  public java.lang.Throwable getCause();
    descriptor: ()Ljava/lang/Throwable;
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: getfield      #13                 // Field cause:Ljava/lang/Throwable;
         4: areturn
}


Changes in the constants pool cause changes in the code (indices change), which causes different MD5 checksums. This means that signatures will not match any more.

This started happening in 3.6.0-beta01 because we are using Jetifier with copyUnmodifiedLibsAlso=true flag (previously it was false).

@Chris You can work around this by updating gradle.properties with:

android.jetifier.blacklist=.*bcprov.*

This will avoid Jetifying the bouncycastle library.

Thanks Ivan!

Pending fix in: https://android-review.googlesource.com/c/platform/frameworks/support/+/1151460

did this fix make it in for 3.6?

Jetifier 1.0.0-beta08 was released and has a fix for this.

To avoid waiting for new Android Studio you can workaround this in the meantime by forcing usage of a new version of jetifier in gradle:
buildscript {
   dependencies {
       classpath 'com.android.tools.build:gradle:x.y.z'
       classpath 'com.android.tools.build.jetifier:jetifier-processor:1.0.0-beta08' // <- Add this
   }
}

Jetifier 1.0.0-beta08, which has the fix for this issue, will be integrated into AGP 3.6.0-rc01 and 4.0.0-alpha06: https://android.googlesource.com/platform/tools/base/+/ef5e56702b626b6be642ecce7d0078b7b1a81bcb

I can't repro the issue yet. If I remove my synchronization code, I can reproduce it immediately (basically, calling isTrackingEnabled while JankStats is issuing frame data on another thread causes the problem). But synchronizing those code blocks on the window object handles that problem. I can't see what else might be happening that would trigger the bug. If anyone has more information on how your app interacts with JankStats (creation, multiple instances, enabling/disabling... anything), it would help.

Hmm, I was just able to force it to happen by doing two things:
- I sent the data directly on the internal thread that receives FrameMetrics info (instead of sending it through the executor, which is the case in alpha02)
- In the OnFrameListener code, I changed the value of isTrackingEnabled

This combination makes it possible to change the delegates list (as a result of isTrackingEnabled) while the list is being iterated (in the FrmaeMetrics listener, which is sending out the frame data to the listeners). Because they are happening on the same thread, the synchronization block doesn't prevent both of these from happening concurrently; it's just a nested synchronization on the same item in the same thread.

This case is pretty interesting because an upcoming change will remove the Executor from the JankStats constructor, moving to a model where data is sent directly on the internal thread, thus skipping the allocations that currently take place.
But this means anything that happens on that thread to change the delegators list (as above) will trip over this problem.
So I'll work on this. But in the meantime:
Does the above match what your app is doing?
For one thing, are you changing isTrackingEnabled in onFrameListener callback?
And what are you using for an Executor? Your version of the library still uses an Executor, but maybe you are setting it up in an unexpected way.

okay, more progress. There are simple ways to create an Executor that runs on the calling thread (I assume that's what you are doing in your app?).
Then it's just a matter of modifying the list of listeners in the listener callback. (again, I assume that is what you are doing?)

I am working on a fix (not sure yet what form it will take).
In the meantime, if you want to fix your app before the fix is available, avoid calling JankStats.isTrackingEnabled in your listener; that's the root of the problem, because it is modifying the listener list while that list is being processed.
(If any of my assumptions are not correct, please give me more details about what your app is actually doing; this is the only thing I can come up with so far which would cause this exception)

Thanks for the information. Can you also tell me how you create the Executor required to create the JankStats instance?

I'm not able to reproduce the error as described in #12. That approach to instantiation and logging is essentially what is done in the JankLoggingActivity test app in the metrics-integration-tests module. The lifecycle callbacks are slightly different there, but I changed the code to use LifecycleCallbacks (onActivityStarted/Paused/Resumed) instead and still don't see the issue. I also tried using an in-thread Executor (as mentioned in my comments above) and still didn't get the problem to occur.

I need to fix the other issue I found anyway, since changing tracking in a listener is definitely broken, but I can't tell what else could be happening besides that that is causing the problem with your app.

A couple of things that would help here:
- What kind of executor are you creating?
- What release(s) are you seeing the exception on?
- Do you see the exception every time you run the app? Is there anything in particular that triggers it (like going to the background or coming back to the foreground)?

Project: platform/frameworks/support
Branch: androidx-main

commit f6e347fa7bbb4ea5a688d3a0af192b2595db2fae
Author: Chet Haase <chet@google.com>
Date:   Fri Jul 01 10:11:09 2022

    Fix ConcurrentModificationException bug
   
    It is possible to add/remove items from the delegator list (in one code
    path) while iterating through those items (in an unrelated code path). This
    can lead to crashes.
   
    The fix was to send all delegator lists through a single, common object.
    Iteration and modifications only happen in a synchronized block on that object.
    To prevent reentrant issues (where a callback on the thread doing the
    iteration can add/remove items and thus succeed in both synchronized blocks),
    adds/removed are gated to avoid having them happen during iteration, postponing
    those operations until the outer iteration is complete.
   
    Bug: 236612357
    Test: Added test to JankStatsTest to verify bug and fix
    RelNote: "Fixed ConcurrentModificationException bug where the list of
    listener delegates was being modified while it was also being iterated through
    to send per-frame data."
   
    Change-Id: Ib769386f18e51dc6b58c935b42c5b8566c644abc

M       metrics/metrics-performance/src/main/java/androidx/metrics/performance/JankStatsApi24Impl.kt
M       metrics/metrics-performance/src/main/java/androidx/metrics/performance/JankStatsApi16Impl.kt
M       metrics/metrics-performance/src/androidTest/java/androidx/metrics/performance/test/JankStatsTest.kt

https://android-review.googlesource.com/2143080

I'm still unable to reproduce the issue as described in the above comments (eg, #16). The fix submitted in #18 addresses another issue that is possible to run into, and it's possible that that fix solidified the concurrency logic in general (it definitely simplified it and resolved the nested issue described in #10). You could try running with the latest snapshot which includes this fix, or wait for alpha03 (not yet scheduled, so at least 3 weeks out).

If you are still seeing the issue with the above change, please reopen. Anything you can tell me about the situation would help; I need to reproduce the problem to make any progress on it. (including, for example, the Android release(s) you are seeing the issue on; that's what I was asking for in #15. But mostly I need steps to reproduce).