1041906 - Replace suspicious use of non-final lock object in GeckoThread with AtomicReference.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chris Kitching [:ckitching]]

Attachment: finalDodgyLock.patch

Ran into this while doing preparatory work for the overlays backend.

GeckoThread uses a non-final lock object and three methods to implement to manage sLaunchState.

Let's just use an AtomicReference. (Which I think is equivalent...)

Try:
https://tbpl.mozilla.org/?tree=Try&rev=2c8cae1c6bfa

Comment 1

[Chris Kitching [:ckitching]]

Try again:
https://tbpl.mozilla.org/?tree=Try&rev=461e2d3855b6

... Still no source level 7, I see.

Comment 2

[Richard Newman [:rnewman]]

Comment on attachment 8460036 [details] [diff] [review]
finalDodgyLock.patch

Review of attachment 8460036 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me (modulo Java7 language feature?).

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8460036 [details] [diff] [review]
finalDodgyLock.patch

Review of attachment 8460036 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: mobile/android/base/GeckoThread.java
@@ +39,2 @@
>  
> +    private static AtomicReference<LaunchState> sLaunchState = new AtomicReference<>(LaunchState.Launching);

Nit: two lines

Comment 4

[Chris Kitching [:ckitching]]

Don't land the version with the Java 7 feature. It won't compile. Landing the right one (plus nit):

https://hg.mozilla.org/integration/fx-team/rev/109d19bf305c

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/109d19bf305c