From 5ca206ffdf1a26086d8cd7397a46d82de9aca435 Mon Sep 17 00:00:00 2001
From: iusmac <iusico.maxim@libero.it>
Date: Tue, 20 Aug 2024 19:33:23 +0200
Subject: [PATCH] fix: address highly critical errorprone issues
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

> Task :compileDebugJavaWithJavac
• 1 src/com/github/iusmac/sevensim/scheduler/SubscriptionScheduleEntity.java:104: error: [EqualsHashCode] Classes that override equals should also override hashCode.
    public boolean equals(final Object o) {
                   ^
    (see https://errorprone.info/bugpattern/EqualsHashCode)

• 2 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:71: error: [GuardedBy] Invalid @GuardedBy expression: static member guarded by instance
    private static Locale sDefaultLocaleCache;
                          ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 3 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:73: error: [GuardedBy] Invalid @GuardedBy expression: static member guarded by instance
    private static String[] sDaysOfWeekNarrowStrings;
                            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 4 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:269: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
        if (sDaysOfWeekNarrowStrings == null || !loc.equals(sDefaultLocaleCache)) {
            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 5 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:269: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
        if (sDaysOfWeekNarrowStrings == null || !loc.equals(sDefaultLocaleCache)) {
                                                            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 6 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:270: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
            sDaysOfWeekNarrowStrings = DateFormatSymbols.getInstance(loc)
            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 7 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:272: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
            sDefaultLocaleCache = loc;
            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 8 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:274: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
        return sDaysOfWeekNarrowStrings[dayOfWeek];
               ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 9 src/com/github/iusmac/sevensim/telephony/Subscription.java:143: error: [BoxedPrimitiveEquality] Comparison using reference equality instead of value equality. Reference equality of boxed primitive types is usually not useful, as they are value objects, and it is bug-prone, as instances are cached for some values but not others.
            && mKeepDisabledAcrossBoots == subToCompare.mKeepDisabledAcrossBoots;
                                        ^
    (see https://errorprone.info/bugpattern/BoxedPrimitiveEquality)
  Did you mean '&& Objects.equals(mKeepDisabledAcrossBoots, subToCompare.mKeepDisabledAcrossBoots);' or '&& mKeepDisabledAcrossBoots.equals(subToCompare.mKeepDisabledAcrossBoots);'?

• 10 src/com/github/iusmac/sevensim/telephony/TelephonyController.java:369: error: [GuardedBy] This access should be guarded by 'TelephonyController.this', which is not currently held
            final Subscription sub = BundleCompat.getParcelable(mRequestMetadata, KEY_SUBSCRIPTION,
                                                                ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 11 src/com/github/iusmac/sevensim/telephony/PinStorage.java:55: error: [GuardedBy] Invalid @GuardedBy expression: static member guarded by instance
    private static long sLastKeystoreAuthTimestamp;
                        ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 12 src/com/github/iusmac/sevensim/telephony/PinStorage.java:381: error: [GuardedBy] This access should be guarded by 'PinStorage'; instead found: 'this'
        final long authTimeout = sLastKeystoreAuthTimestamp == 0 ? 0 : sLastKeystoreAuthTimestamp +
                                 ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 13 src/com/github/iusmac/sevensim/telephony/PinStorage.java:381: error: [GuardedBy] This access should be guarded by 'PinStorage'; instead found: 'this'
        final long authTimeout = sLastKeystoreAuthTimestamp == 0 ? 0 : sLastKeystoreAuthTimestamp +
                                                                       ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 14 src/com/github/iusmac/sevensim/telephony/PinStorage.java:394: error: [GuardedBy] This access should be guarded by 'PinStorage'; instead found: 'PinStorage.class'
        sLastKeystoreAuthTimestamp = timestamp;
        ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 15 src/com/github/iusmac/sevensim/ui/components/ItemAdapter.java:456: error: [EqualsHashCode] Classes that override equals should also override hashCode.
        public boolean equals(final Object o) {
                       ^
    (see https://errorprone.info/bugpattern/EqualsHashCode)

Signed-off-by: iusmac <iusico.maxim@libero.it>
---
 .../iusmac/sevensim/scheduler/DaysOfWeek.java |  7 ++--
 .../scheduler/SubscriptionScheduleEntity.java |  7 ++++
 .../iusmac/sevensim/telephony/PinStorage.java | 14 ++++---
 .../sevensim/telephony/SimPinFeeder.java      | 36 +++++++++-------
 .../sevensim/telephony/Subscription.java      |  2 +-
 .../telephony/TelephonyController.java        | 41 ++++++++++++-------
 .../sevensim/ui/components/ItemAdapter.java   |  6 +++
 7 files changed, 73 insertions(+), 40 deletions(-)

diff --git a/src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java b/src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java
index a37ca96..2f2b0f2 100644
--- a/src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java
+++ b/src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java
@@ -67,9 +67,10 @@ public final class DaysOfWeek implements Iterable<Integer>, Comparable<DaysOfWee
     /** The sum of all days of the week bit masks. */
     private static final int ALL_DAYS_OF_WEEK_BITS = 0x7f;
 
-    @GuardedBy("this")
+    @GuardedBy("DaysOfWeek.class")
     private static Locale sDefaultLocaleCache;
-    @GuardedBy("this")
+
+    @GuardedBy("DaysOfWeek.class")
     private static String[] sDaysOfWeekNarrowStrings;
 
     /** An encoded form of a weekly repeat schedule. */
@@ -266,7 +267,7 @@ public String getDisplayName(final @DayOfWeek int dayOfWeek, final boolean useLo
      * @param dayOfWeek Any of {@link DayOfWeek} values.
      * @return Single-character weekday name; e.g.: 'S', 'M', 'T', 'W', 'T', 'F', 'S'.
      */
-    public synchronized @NonNull String getNarrowDisplayName(final @DayOfWeek int dayOfWeek) {
+    public synchronized static @NonNull String getNarrowDisplayName(final @DayOfWeek int dayOfWeek) {
         final Locale loc = Locale.getDefault();
         if (sDaysOfWeekNarrowStrings == null || !loc.equals(sDefaultLocaleCache)) {
             sDaysOfWeekNarrowStrings = DateFormatSymbols.getInstance(loc)
diff --git a/src/com/github/iusmac/sevensim/scheduler/SubscriptionScheduleEntity.java b/src/com/github/iusmac/sevensim/scheduler/SubscriptionScheduleEntity.java
index a74ba95..58034e0 100644
--- a/src/com/github/iusmac/sevensim/scheduler/SubscriptionScheduleEntity.java
+++ b/src/com/github/iusmac/sevensim/scheduler/SubscriptionScheduleEntity.java
@@ -9,6 +9,7 @@
 import androidx.room.PrimaryKey;
 
 import java.time.LocalTime;
+import java.util.Objects;
 
 /**
  * This class is a data transfer object (DTO), representing a weekly repeat schedule for managing
@@ -115,6 +116,12 @@ public boolean equals(final Object o) {
             && compareTo.mTime.equals(mTime);
     }
 
+    @Override
+    public int hashCode() {
+        return Objects.hash(mId, mSubscriptionId, mSubscriptionEnabled, mLabel, mEnabled,
+                mDaysOfWeek, mTime);
+    }
+
     @Override
     public String toString() {
         return "SubscriptionScheduleEntity {"
diff --git a/src/com/github/iusmac/sevensim/telephony/PinStorage.java b/src/com/github/iusmac/sevensim/telephony/PinStorage.java
index ec1d25e..ba13f45 100644
--- a/src/com/github/iusmac/sevensim/telephony/PinStorage.java
+++ b/src/com/github/iusmac/sevensim/telephony/PinStorage.java
@@ -52,7 +52,7 @@ public final class PinStorage {
      * The {@link SystemClock#elapsedRealtime()}-based time, when the hardware-backed KeyStore has
      * been unlocked.
      */
-    @GuardedBy("this")
+    @GuardedBy("PinStorage.class")
     private static long sLastKeystoreAuthTimestamp;
 
     /** The default duration, in seconds, of the user authentication bound secret key. */
@@ -379,11 +379,13 @@ private void deleteSecretKey() {
      * Check whether the user should be authenticated with their credentials in order to unlock the
      * hardware-backed KeyStore for further crypto operations.
      */
-    public synchronized boolean isAuthenticationRequired() {
-        final long authTimeout = sLastKeystoreAuthTimestamp == 0 ? 0 : sLastKeystoreAuthTimestamp +
-            DEFAULT_AUTHENTICATION_VALIDITY_DURATION_SECONDS * 1000L;
-        final boolean isKeystoreAuthExpired = authTimeout - SystemClock.elapsedRealtime() < 0;
-        return mKeyguardManagerLazy.get().isDeviceSecure() && isKeystoreAuthExpired;
+    public boolean isAuthenticationRequired() {
+        synchronized (PinStorage.class) {
+            final long authTimeout = sLastKeystoreAuthTimestamp == 0 ? 0 :
+                sLastKeystoreAuthTimestamp + DEFAULT_AUTHENTICATION_VALIDITY_DURATION_SECONDS * 1000L;
+            final boolean isKeystoreAuthExpired = authTimeout - SystemClock.elapsedRealtime() < 0;
+            return mKeyguardManagerLazy.get().isDeviceSecure() && isKeystoreAuthExpired;
+        }
     }
 
     /**
diff --git a/src/com/github/iusmac/sevensim/telephony/SimPinFeeder.java b/src/com/github/iusmac/sevensim/telephony/SimPinFeeder.java
index 47e37ed..36cf473 100644
--- a/src/com/github/iusmac/sevensim/telephony/SimPinFeeder.java
+++ b/src/com/github/iusmac/sevensim/telephony/SimPinFeeder.java
@@ -168,22 +168,28 @@ public void run() {
                 // Wait to be notified about new SIM state changes or finish on timeout
                 synchronized (simStatusChangedListener) {
                     if (!mSimStatusChanged) {
-                        try {
-                            // Note that for reliability, we want to wait a maximum of 10 seconds
-                            // for more SIM card state change events before finishing. Normally,
-                            // these events are delivered within a second, but in some edge cases,
-                            // such as under high memory pressure, delivery may be delayed even by
-                            // 2-3 seconds. This also serves as a "window" to give time to the SIM
-                            // cards to enter the PIN state in case we started slightly earlier
-                            simStatusChangedListener.wait(10_000L);
-                            if (!mSimStatusChanged) {
-                                // Timed out. No more events
-                                break threadLoop;
-                            }
-                        } catch (InterruptedException ignored) {
-                            if (mReleased) {
-                                break threadLoop;
+                        long nowMillis = System.currentTimeMillis();
+                        // Note that for reliability, we want to wait a maximum of 10 seconds for
+                        // more SIM card state change events before finishing. Normally, these
+                        // events are delivered within a second, but in some edge cases, such as
+                        // under high memory pressure, delivery may be delayed even by 2-3 seconds.
+                        // This also serves as a "window" to give time to the SIM cards to enter the
+                        // PIN state in case we started slightly earlier
+                        final long deltaMillis = nowMillis + 10_000L;
+                        do {
+                            try {
+                                simStatusChangedListener.wait(deltaMillis - nowMillis);
+                            } catch (InterruptedException ignored) {
+                                if (mReleased) {
+                                    break threadLoop;
+                                }
                             }
+                            nowMillis = System.currentTimeMillis();
+                        } while (!mSimStatusChanged && nowMillis < deltaMillis);
+
+                        if (!mSimStatusChanged) {
+                            // Timed out. No more events
+                            break threadLoop;
                         }
                     }
                     mSimStatusChanged = false;
diff --git a/src/com/github/iusmac/sevensim/telephony/Subscription.java b/src/com/github/iusmac/sevensim/telephony/Subscription.java
index 71e8b8a..d0fabf9 100644
--- a/src/com/github/iusmac/sevensim/telephony/Subscription.java
+++ b/src/com/github/iusmac/sevensim/telephony/Subscription.java
@@ -140,7 +140,7 @@ public boolean equals(final Object o) {
             && mName.equals(subToCompare.mName)
             && mLastActivatedTime.equals(subToCompare.mLastActivatedTime)
             && mLastDeactivatedTime.equals(subToCompare.mLastDeactivatedTime)
-            && mKeepDisabledAcrossBoots == subToCompare.mKeepDisabledAcrossBoots;
+            && Objects.equals(mKeepDisabledAcrossBoots, subToCompare.mKeepDisabledAcrossBoots);
     }
 
     @Override
diff --git a/src/com/github/iusmac/sevensim/telephony/TelephonyController.java b/src/com/github/iusmac/sevensim/telephony/TelephonyController.java
index e8319ca..03e6adf 100644
--- a/src/com/github/iusmac/sevensim/telephony/TelephonyController.java
+++ b/src/com/github/iusmac/sevensim/telephony/TelephonyController.java
@@ -171,11 +171,22 @@ public void setSimState(final int slotIndex, final boolean enabled,
 
             setSimPowerStateForSlot(slotIndex, simStateInt(enabled));
 
-            try {
-                wait(SET_SIM_POWER_STATE_REQUEST_TIMEOUT_MILLIS);
-            } catch (InterruptedException e) {
-                mLogger.w(logPrefix + "Acquire wait interrupted.");
-            }
+            // Wait for the SIM power request to complete or timeout
+            long nowMillis = System.currentTimeMillis();
+            final long deadlineMillis = nowMillis + SET_SIM_POWER_STATE_REQUEST_TIMEOUT_MILLIS;
+            do {
+                try {
+                    wait(deadlineMillis - nowMillis);
+                } catch (InterruptedException e) {
+                    mLogger.w(logPrefix + "Acquire wait interrupted.");
+                    break;
+                }
+                if (mRequestMetadata.isEmpty()) {
+                    // Request metadata consumed, so we break here as this wasn't a spurious wakeup
+                    break;
+                }
+                nowMillis = System.currentTimeMillis();
+            } while (nowMillis < deadlineMillis);
 
             if (Utils.IS_OLDER_THAN_S) {
                 mSubscriptions.removeOnSimStatusChangedListener(mSimStatusChangedListener);
@@ -369,17 +380,17 @@ public void onSimStatusChanged(final int slotIndex,
                 default: return;
             }
 
-            final Subscription sub = BundleCompat.getParcelable(mRequestMetadata, KEY_SUBSCRIPTION,
-                    Subscription.class);
-
-            if (sub.getSlotIndex() != slotIndex) {
-                // Since we're listening to state mutations of all available SIM cards, we can
-                // hypothetically receive a concurrent update for a different SIM card than the one
-                // whose SIM power state we expect to change
-                return;
-            }
-
             synchronized (TelephonyController.this) {
+                final Subscription sub = BundleCompat.getParcelable(mRequestMetadata,
+                        KEY_SUBSCRIPTION, Subscription.class);
+
+                if (sub.getSlotIndex() != slotIndex) {
+                    // Since we're listening to state mutations of all available SIM cards, we can
+                    // hypothetically receive a concurrent update for a different SIM card than the
+                    // one whose SIM power state we expect to change
+                    return;
+                }
+
                 handleOnSetSimPowerStateForSlotFinished(state);
                 TelephonyController.this.notifyAll();
             }
diff --git a/src/com/github/iusmac/sevensim/ui/components/ItemAdapter.java b/src/com/github/iusmac/sevensim/ui/components/ItemAdapter.java
index 8b2ad4f..aa06af6 100644
--- a/src/com/github/iusmac/sevensim/ui/components/ItemAdapter.java
+++ b/src/com/github/iusmac/sevensim/ui/components/ItemAdapter.java
@@ -48,6 +48,7 @@
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Objects;
 
 /**
  * Base adapter class for displaying a collection of items. Provides functionality for
@@ -455,6 +456,11 @@ public void onRestoreInstanceState(Bundle bundle) {
             // for subclassers
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(itemId, item);
+        }
+
         @Override
         public boolean equals(final Object o) {
             if (o == this) return true;