Fixed NN_NAKED_NOTIFY Spotbugs Issues (#4417)

Author: shai-almog

.github/scripts/generate-quality-report.py

@@ -815,6 +815,7 @@ def main() -> None:
             "NM_CONFUSING",
             "NM_FIELD_NAMING_CONVENTION",
             "NM_METHOD_NAMING_CONVENTION",
+            "NN_NAKED_NOTIFY",
             "NO_NOTIFY_NOT_NOTIFYALL",
             "NP_LOAD_OF_KNOWN_NULL_VALUE",
             "NP_BOOLEAN_RETURN_NULL",
@@ -851,6 +852,8 @@ def _is_exempt(f: Finding) -> bool:
                 return True
             if f.rule == "URF_UNREAD_FIELD" and "GridBagLayoutInfo" in loc:
                 return True
+            if f.rule == "NN_NAKED_NOTIFY" and "Display.java" in loc:
+                return True
             return False
 
 

CodenameOne/src/com/codename1/capture/Capture.java

@@ -257,8 +257,8 @@ public void actionPerformed(ActionEvent evt) {
             } else {
                 url = (String) evt.getSource();
             }
-            completed = true;
             synchronized (this) {
+                completed = true;
                 this.notifyAll();
             }
         }

CodenameOne/src/com/codename1/components/WebBrowser.java

@@ -154,8 +154,8 @@ protected void readResponse(InputStream input) throws IOException {
                         if (callback != null) {
                             callback.streamReady(input, docInfo);
                         } else {
-                            response[0] = input;
                             synchronized (LOCK) {
+                                response[0] = input;
                                 LOCK.notifyAll();
                             }
                         }

CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java

@@ -7177,13 +7177,9 @@ protected final void pushReceived(String data) {
      * Sets the frequency for polling the server in case of polling based push notification
      *
      * @param freq the frequency in milliseconds
+     * @deprecated we no longer support push polling
      */
     public void setPollingFrequency(int freq) {
-        if (callback != null && pollingThreadRunning) {
-            synchronized (callback) {
-                callback.notifyAll();
-            }
-        }
     }
 
     /**

CodenameOne/src/com/codename1/io/NetworkManager.java

@@ -330,8 +330,8 @@ public void shutdown() {
                 }
             }
         }
-        networkThreads = null;
         synchronized (LOCK) {
+            networkThreads = null;
             LOCK.notifyAll();
         }
 
@@ -970,10 +970,10 @@ public void run() {
                     if (!runCurrentRequest(currentRequest)) {
                         continue;
                     }
-                    currentRequest = null;
 
                     // wakeup threads waiting for the completion of this network operation
                     synchronized (LOCK) {
+                        currentRequest = null;
                         LOCK.notifyAll();
                     }
                 } else {

CodenameOne/src/com/codename1/ui/Display.java

@@ -506,9 +506,8 @@ public static void init(Object m) {
      * Notice that minimize (being a Codename One method) MUST be invoked before invoking this method!
      */
     public static void deinitialize() {
-
-        INSTANCE.codenameOneRunning = false;
         synchronized (lock) {
+            INSTANCE.codenameOneRunning = false;
             lock.notifyAll();
         }
     }
@@ -4684,6 +4683,7 @@ public String getDatabasePath(String databaseName) {
      * Sets the frequency for polling the server in case of polling based push notification
      *
      * @param freq the frequency in milliseconds
+     * @deprecated we no longer support push polling
      */
     public void setPollingFrequency(int freq) {
         impl.setPollingFrequency(freq);

CodenameOne/src/com/codename1/ui/RunnableWrapper.java

@@ -106,8 +106,8 @@ public void run() {
             switch (type) {
                 case 0:
                     internal.run();
-                    done = true;
                     synchronized (Display.lock) {
+                        done = true;
                         Display.lock.notifyAll();
                     }
                     break;

CodenameOne/src/com/codename1/ui/html/AsyncDocumentRequestHandlerImpl.java

@@ -155,8 +155,8 @@ protected void readResponse(InputStream input) throws IOException {
             if (callback != null) {
                 callback.streamReady(input, docInfo);
             } else {
-                response[0] = input;
                 synchronized (LOCK) {
+                    response[0] = input;
                     LOCK.notifyAll();
                 }
             }

maven/core-unittests/spotbugs-exclude.xml

@@ -152,6 +152,15 @@
         <Bug pattern="SF_SWITCH_FALLTHROUGH" />
     </Match>
 
+    <!--
+    This rule is a bit broad for the complex threading logic in Display.
+    Ideally, this code should be rewritten, but it's battle tested and deep.
+    -->
+    <Match>
+        <Class name="~com\.codename1\.ui\.Display.*" />
+        <Bug pattern="NN_NAKED_NOTIFY" />
+    </Match>
+
     <!-- Excluding since the class explicitly compares interned strings -->
     <Match>
         <Class name="~com\.codename1\.io\.ConnectionRequest.*" />