1889808 - org.mozilla.fenix.messaging.state.MessagingMiddleware$onMessagedDisplayed Crash in [@ java.lang.ArrayIndexOutOfBoundsException: at java.util.ArrayList.set(ArrayList.java)]

Status: RESOLVED FIXED

(Fenix :: Experimentation and Telemetry, defect)

Description

[Chris Peterson [:cpeterson]]

Crash report: https://crash-stats.mozilla.org/report/index/8c070069-f86c-4ba0-b3a3-e58e10240404

These crashes might be a regression in Fx 125. The earliest crash reports are from Fx 125.0a1, but it's hard to say when the bug landed because the crash volume is very low.

Exception: java.lang.ArrayIndexOutOfBoundsException
Message: length=1; index=-1

Frame   Module  Function  Source
0   java.util.ArrayList   set   ArrayList.java:483
1   org.mozilla.fenix.messaging.state.MessagingMiddleware$onMessagedDisplayed$1   invokeSuspend   MessagingMiddleware.kt:169
2   kotlin.coroutines.jvm.internal.BaseContinuationImpl   resumeWith  ContinuationImpl.kt:9
3   kotlinx.coroutines.DispatchedTask   run   DispatchedTask.kt:111
4   kotlinx.coroutines.internal.LimitedDispatcher$Worker  run   LimitedDispatcher.kt:4
5   kotlinx.coroutines.scheduling.TaskImpl  run   Tasks.kt:3
6   kotlinx.coroutines.scheduling.CoroutineScheduler$Worker   run   CoroutineScheduler.kt:98

Comment 1

[Chris Peterson [:cpeterson]]

Roger, do you think this MessagingMiddleware crash could be a regression from your fix for ReducerChainBuilder crash bug 1882123?

Comment 2

[Roger Yang [:royang]]

It is possible my change uncovered a race that used to prevent this crash. A quick investigation seems to indicate there are some missing handling around where we crashed. I'll investigate.

Comment 3

[Donal Meehan [:dmeehan]]

(In reply to Roger Yang [:royang] from comment #2)

It is possible my change uncovered a race that used to prevent this crash. A quick investigation seems to indicate there are some missing handling around where we crashed. I'll investigate.

:royang any update from your investigation?
Have you confirmed or ruled out Bug 1882123 as the regressor?
We still have two planned dot releases left for Fx126 schedule, if you think you might have a fix in time?

Comment 4

[Roger Yang [:royang]]

I did ruled out Bug 1882123 as a regressor but I think I have a fix for this. I'll put it up today. Thanks

Comment 5

[Roger Yang [:royang]]

Attachment: Bug 1889808 - Do not attempt to update a message that was removed.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aab46451484b
Do not attempt to update a message that was removed. r=android-reviewers,boek

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/aab46451484b

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:royang, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox127 to wontfix.

For more information, please visit BugBot documentation.

Comment 9

[Donal Meehan [:dmeehan]]

To add to comment 8, please consider adding a release uplift request on this also.
Depending on the risk, we could include this in the planned Fx126 dot release.

Comment 10

[Roger Yang [:royang]]

(In reply to Donal Meehan [:dmeehan] from comment #9)

To add to comment 8, please consider adding a release uplift request on this also.
Depending on the risk, we could include this in the planned Fx126 dot release.

Sounds good. I'll put up the uplift request. The change is pretty safe.

Comment 11

[Roger Yang [:royang]]

Comment on attachment 9402496 [details]
Bug 1889808 - Do not attempt to update a message that was removed.

Beta/Release Uplift Approval Request

• User impact if declined: In some cases a race condition causes the browser to crash
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This fix only catches the race condition and prevents crashing.
• String changes made/needed:
• Is Android affected?: Yes

Comment 12

[Pascal Chevrel:pascalc]

Comment on attachment 9402496 [details]
Bug 1889808 - Do not attempt to update a message that was removed.

Approved for 127 beta 6, thanks.

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/0005fc3c8ae9

Comment 14

[Donal Meehan [:dmeehan]]

Comment on attachment 9402496 [details]
Bug 1889808 - Do not attempt to update a message that was removed.

Approved for 126.0.1

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/f91e0056c976