amplify-android icon indicating copy to clipboard operation
amplify-android copied to clipboard

ArrayIndexOutOfBoundsException caused by TransferStatusUpdater.unregisterListener$lambda$15

Open yaroslav-v opened this issue 1 year ago • 8 comments

Before opening, please confirm:

Language and Async Model

Java

Amplify Categories

Storage

Gradle script dependencies

// Put output below this line
implementation "com.amplifyframework:core:2.14.7"
implementation "com.amplifyframework:aws-storage-s3:2.14.7"
implementation "com.amplifyframework:aws-auth-cognito:2.14.7"

Environment information

# Put output below this line
------------------------------------------------------------
Gradle 8.1.1
------------------------------------------------------------

Build time:   2023-04-21 12:31:26 UTC
Revision:     1cf537a851c635c364a4214885f8b9798051175b

Kotlin:       1.8.10
Groovy:       3.0.15
Ant:          Apache Ant(TM) version 1.10.11 compiled on July 10 2021
JVM:          1.8.0_201 (Oracle Corporation 25.201-b09)
OS:           Mac OS X 10.16 x86_64

Please include any relevant guides or documentation you're referencing

No response

Describe the bug

Hi there!

We have a few (3 at the moment) reports on Firebase with this crash. The issue appears on Android 13 only, affected devices include Samsung and Xiaomi.

The issue is quite rare and, unfortunately, there are no steps to reproduce it.

I believe you may have more clues why this might happen.

Reproduction steps (if applicable)

No response

Code Snippet

// Initialization in Application class

Amplify.Logging.disable();

Amplify.addPlugin(new AWSCognitoAuthPlugin());
Amplify.addPlugin(new AWSS3StoragePlugin());

AmplifyConfiguration amplifyConfig = AmplifyConfiguration.builder(context)
		.devMenuEnabled(false)
		.build();
Amplify.configure(amplifyConfig, context);


// Uploading, somewhere in the app

DateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ", Locale.US);
dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));

Map<String, String> metadata = new ArrayMap<>(4);
metadata.put("date", dateFormat.format(date));
metadata.put("session_uuid", uid);
metadata.put("token", authToken);
metadata.put("service", serviceName);

StorageUploadFileOptions options =
		StorageUploadFileOptions.builder()
				.accessLevel(StorageAccessLevel.PUBLIC)
				.contentType(mimeType)
				.metadata(metadata)
				.build();

final File file = new File(path);
if (file.canRead()) {
	Amplify.Storage.uploadFile(
			file.getName(),
			file,
			options,
			result -> {
				// write success logs
			},
			failure -> {
				// write failure logs
			}
	);
}

Log output

// Put your logs below this line
Fatal Exception: java.lang.ArrayIndexOutOfBoundsException
length=1; index=1
	java.util.ArrayList.remove (ArrayList.java:631)
	com.amplifyframework.storage.s3.transfer.TransferStatusUpdater.unregisterListener$lambda$15 (TransferStatusUpdater.kt:214)
	android.os.Handler.handleCallback (Handler.java:942)
	android.os.Handler.dispatchMessage (Handler.java:99)
	android.os.Looper.loopOnce (Looper.java:211)
	android.os.Looper.loop (Looper.java:300)
	android.app.ActivityThread.main (ActivityThread.java:8410)
	java.lang.reflect.Method.invoke (Method.java)
	com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:559)
	com.android.internal.os.ZygoteInit.main (ZygoteInit.java:954)

amplifyconfiguration.json

No response

GraphQL Schema

// Put your schema below this line


Additional information and screenshots

No response

yaroslav-v avatar Dec 25 '23 17:12 yaroslav-v

Thank you for the bug report. A member of our team will take a look.

tylerjroach avatar Dec 27 '23 14:12 tylerjroach

@Synchronized
fun unregisterListener(transferRecordId: Int, transferListener: TransferListener) {
    mainHandler.post {
        transferStatusListenerMap[transferRecordId]?.remove(transferListener)
    }
}

There appears to be a few faulty threading assumptions used here. This method is synchronized, but immediately calls mainHandler.post { } which means the actual work wont be done in the synchronized block.

There are some additional attempts at safeguard, such as transferStatusListenerMap being a ConcurrentHashMap. This area needs refactoring to ensure better thread safety.

tylerjroach avatar Dec 27 '23 14:12 tylerjroach

Wanted to add some additional research notes here.

Crash is java.util.ArrayList.remove (ArrayList.java:631) which shows the crash is coming from the remove(transferListener) call once a list is grabbed from the ConcurrentHashMap, transferStatusListenerMap.

Under the hood, this Kotlin helper method first does an indexOf(element), then a removeAt(index). My guess is that another area of code would have had to have shortened the list after indexOf(element) is called and before removeAt(index) is called.

public override fun remove(element: E): Boolean {
    val index = indexOf(element)
    if (index == -1) return false
    removeAt(index)
    return true
}

While this block in TransferStatusUpdater is a bit misguided because it attempts to synchronize and then goes out to a different thread, this is the only area of the code that removes items from the list. All the removes are posted to the same (main) thread so I would not expect any threading issues at this point.

@Synchronized
fun unregisterListener(transferRecordId: Int, transferListener: TransferListener) {
    mainHandler.post {
        transferStatusListenerMap[transferRecordId]?.remove(transferListener)
    }
}

Were still missing something since this crash is being observed, but I'm still not sure how this is happening.

tylerjroach avatar Feb 13 '24 15:02 tylerjroach

I am getting the same error in firebase, here is the data

image

Fatal Exception: java.lang.ArrayIndexOutOfBoundsException: length=1; index=1
       at java.util.ArrayList.remove(ArrayList.java:631)
       at com.amplifyframework.storage.s3.transfer.TransferStatusUpdater.unregisterListener$lambda$15(TransferStatusUpdater.kt:214)

xaluxs avatar Mar 15 '24 00:03 xaluxs

@xaluxs Thanks for the additional detail. How many times have you seen this crash. Any other devices?

tylerjroach avatar Mar 15 '24 13:03 tylerjroach

Hi guys!

I just want to give you some additional context. For the last 30 days this issue was registered for 13 users in Firebase.

The issue appears on Android 12-14, affected devices are Samsung (Galaxy A14, Galaxy S21 Ultra, Galaxy S21 FE etc.), Motorola (Moto G Stylus (2023)), Sony (Xperia 5 III), Ragentek - FIH Foxconn (WP23), Google.

The stack trace is the same for all cases.

AWS Amplify v2.14.11

yaroslav-v avatar Apr 08 '24 09:04 yaroslav-v

@yaroslav-v Thank you for your update, we will take a look and provide updates.

yuhengshs avatar Apr 09 '24 16:04 yuhengshs