Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-48729

FindBugs reports concurrency issues in Lib Task Reactor

XMLWordPrintable

      Task Reactor is a mission-critical component of the Jenkins initialization logic. IMHO we need to investigate the issues and fix/suppress them

      [INFO] --- findbugs-maven-plugin:3.0.5:check (default) @ task-reactor ---
[INFO] BugInstance size is 3
[INFO] Error size is 0
[INFO] Total bugs: 3
[INFO] Inconsistent synchronization of org.jvnet.hudson.reactor.Reactor.executor; locked 50% of time [org.jvnet.hudson.reactor.Reactor$Node, org.jvnet.hudson.reactor.Reactor$Node, org.jvnet.hudson.reactor.Reactor, org.jvnet.hudson.reactor.Reactor, org.jvnet.hudson.reactor.Reactor] Unsynchronized access at Reactor.java:[line 139]Unsynchronized access at Reactor.java:[line 109]Synchronized access at Reactor.java:[line 256]Synchronized access at Reactor.java:[line 273]Synchronized access at Reactor.java:[line 273] IS2_INCONSISTENT_SYNC
[INFO] Inconsistent synchronization of org.jvnet.hudson.reactor.Reactor.fatal; locked 75% of time [org.jvnet.hudson.reactor.Reactor$Node, org.jvnet.hudson.reactor.Reactor$Node, org.jvnet.hudson.reactor.Reactor, org.jvnet.hudson.reactor.Reactor] Unsynchronized access at Reactor.java:[line 119]Synchronized access at Reactor.java:[line 126]Synchronized access at Reactor.java:[line 268]Synchronized access at Reactor.java:[line 269] IS2_INCONSISTENT_SYNC
[INFO] Using notify rather than notifyAll in org.jvnet.hudson.reactor.Reactor$Node.run() [org.jvnet.hudson.reactor.Reactor$Node] At Reactor.java:[line 131] NO_NOTIFY_NOT_NOTIFYALL
[INFO]

          [JENKINS-48729] FindBugs reports concurrency issues in Lib Task Reactor

          Oleg Nenashev created issue -

          Oleg Nenashev added a comment -

          Analysis from greybird in https://github.com/jenkinsci/envinject-api-plugin/pull/5:

          • fatal:
            • the 1 out of 4 occurences where it is not synchronized is when the task sets the field after an exception.
            • as only a single exception is kept, and variable assignment is thread-safe, not a real issue
            • (also synchronization is not really related to the fatal assignment on the locations where it is present)
              executor
          • not synchronized read are from task's runIfPossible (and canRun, which is only called from runIfPossible)
            • executor values are set from execute method, which is synchronized
            • runIfPossible methods are called from
              • addAll and execute, which are synchronized
              • Node.run, under a synchronize block on Reactor.this
            • so again I would say not a real issue
          • notify vs notifyall
            • all threads are waiting for Reactor instance lock.
            • only threads actually capable of doing something are in the list of active tasks
            • in this situation I would say that using notifyAll would lead to waking more threads than needed
            • I'm not sure of the behavior if any external component is taking a lock on the Reactor instance however ?

          Oleg Nenashev added a comment - Analysis from greybird in https://github.com/jenkinsci/envinject-api-plugin/pull/5: fatal: the 1 out of 4 occurences where it is not synchronized is when the task sets the field after an exception. as only a single exception is kept, and variable assignment is thread-safe, not a real issue (also synchronization is not really related to the fatal assignment on the locations where it is present) executor not synchronized read are from task's runIfPossible (and canRun, which is only called from runIfPossible) executor values are set from execute method, which is synchronized runIfPossible methods are called from addAll and execute, which are synchronized Node.run, under a synchronize block on Reactor.this so again I would say not a real issue notify vs notifyall all threads are waiting for Reactor instance lock. only threads actually capable of doing something are in the list of active tasks in this situation I would say that using notifyAll would lead to waking more threads than needed I'm not sure of the behavior if any external component is taking a lock on the Reactor instance however ?

            Unassigned Unassigned
            oleg_nenashev Oleg Nenashev
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: