Various SpotBugs fixes

Author: eregon

mx.truffleruby/spotbugs-filters.xml

@@ -21,6 +21,9 @@
         <Bug pattern="EI_EXPOSE_REP2" />
         <Bug pattern="FE_FLOATING_POINT_EQUALITY" />
         <Bug pattern="NM_CLASS_NOT_EXCEPTION" />
+        <Bug pattern="RC_REF_COMPARISON_BAD_PRACTICE_BOOLEAN" />
+        <Bug pattern="BX_UNBOXING_IMMEDIATELY_REBOXED" /> <!-- used as a way to assert the type -->
+        <Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS" />
       </Or>
     </Not>
   </Match>

src/main/java/org/truffleruby/RubyContext.java

@@ -144,11 +144,16 @@ public class RubyContext {
 
     private static boolean preInitializeContexts = TruffleRuby.PRE_INITIALIZE_CONTEXTS;
 
+    private static boolean isPreInitializingContext() {
+        boolean isPreInitializingContext = preInitializeContexts;
+        preInitializeContexts = false; // Only the first context is pre-initialized
+        return isPreInitializingContext;
+    }
+
     public RubyContext(RubyLanguage language, TruffleLanguage.Env env) {
         Metrics.printTime("before-context-constructor");
 
-        this.preInitializing = preInitializeContexts;
-        RubyContext.preInitializeContexts = false; // Only the first context is pre-initialized
+        this.preInitializing = isPreInitializingContext();
         this.preInitialized = preInitializing;
 
         preInitializationManager = preInitializing ? new PreInitializationManager(this) : null;

src/main/java/org/truffleruby/RubyFileTypeDetector.java

@@ -14,7 +14,6 @@
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Locale;
-import java.util.function.BiConsumer;
 import java.util.regex.Pattern;
 
 import org.jcodings.Encoding;
@@ -80,14 +79,11 @@ public Charset findEncoding(TruffleFile file) throws IOException {
                 if (encodingCommentLine != null) {
                     Rope encodingCommentRope = StringOperations.encodeRope(encodingCommentLine, UTF8Encoding.INSTANCE);
                     Charset[] encodingHolder = new Charset[1];
-                    RubyLexer.parseMagicComment(encodingCommentRope, new BiConsumer<String, Rope>() {
-                        @Override
-                        public void accept(String name, Rope value) {
-                            if (RubyLexer.isMagicEncodingComment(name)) {
-                                Encoding encoding = EncodingManager.getEncoding(value);
-                                if (encoding != null) {
-                                    encodingHolder[0] = encoding.getCharset();
-                                }
+                    RubyLexer.parseMagicComment(encodingCommentRope, (name, value) -> {
+                        if (RubyLexer.isMagicEncodingComment(name)) {
+                            Encoding encoding = EncodingManager.getEncoding(value);
+                            if (encoding != null) {
+                                encodingHolder[0] = encoding.getCharset();
                             }
                         }
                     });

src/main/java/org/truffleruby/aot/TruffleRubySupport.java

@@ -26,6 +26,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import com.oracle.truffle.api.CompilerDirectives;
 import org.jcodings.Encoding;
 import org.jcodings.EncodingDB;
 import org.jcodings.util.ArrayReader;
@@ -73,12 +74,15 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
             try (InputStream is = ArrayReader.class.getResourceAsStream(entry)) {
                 if (is != null) {
                     byte[] buf = new byte[is.available()];
-                    new DataInputStream(is).readFully(buf);
+                    try (DataInputStream dataInputStream = new DataInputStream(is)) {
+                        dataInputStream.readFully(buf);
+                    }
                     jcodingsTables.put(name, buf);
                 } else {
-                    throw new Error("Unable to load Jcodings table " + name);
+                    throw CompilerDirectives.shouldNotReachHere("Unable to load Jcodings table " + name);
                 }
             } catch (IOException e) {
+                throw CompilerDirectives.shouldNotReachHere(e);
             }
         }
         return jcodingsTables;

src/main/java/org/truffleruby/cext/ValueWrapperManager.java

@@ -17,8 +17,10 @@
 
 import org.truffleruby.RubyContext;
 import org.truffleruby.RubyLanguage;
+import org.truffleruby.SuppressFBWarnings;
 import org.truffleruby.cext.ValueWrapperManagerFactory.AllocateHandleNodeGen;
 import org.truffleruby.cext.ValueWrapperManagerFactory.GetHandleBlockHolderNodeGen;
+import org.truffleruby.extra.ffi.Pointer;
 import org.truffleruby.language.NotProvided;
 import org.truffleruby.language.RubyBaseNode;
 
@@ -32,6 +34,7 @@
 import com.oracle.truffle.api.library.ExportLibrary;
 import com.oracle.truffle.api.library.ExportMessage;
 
+@SuppressFBWarnings("VO")
 public class ValueWrapperManager {
 
     static final long UNSET_HANDLE = -2L;
@@ -232,7 +235,7 @@ public boolean isFull() {
         }
 
         public long setHandleOnWrapper(ValueWrapper wrapper) {
-            long handle = getBase() + count * 8;
+            long handle = getBase() + count * Pointer.SIZE;
             wrapper.setHandle(handle, this);
             wrappers[count] = wrapper;
             count++;

src/main/java/org/truffleruby/collections/IntHashMap.java

@@ -331,17 +331,4 @@ public String toString() {
         }
     }
 
-    @Override
-    public Object clone() {
-        IntHashMap<V> newMap = new IntHashMap<>(table.length, loadFactor);
-        for (int i = 0; i < table.length; i++) {
-            Entry<V> entry = table[i];
-            while (entry != null) {
-                newMap.put(entry.getKey(), entry.getValue());
-                entry = entry.next;
-            }
-        }
-        return newMap;
-    }
-
 }

src/main/java/org/truffleruby/core/CoreLibrary.java

@@ -700,6 +700,7 @@ public CoreLibrary(RubyContext context) {
         patchFiles = initializePatching(context);
     }
 
+    @SuppressFBWarnings("SIC")
     private ConcurrentMap<String, Boolean> initializePatching(RubyContext context) {
         defineModule(truffleModule, "Patching");
         final ConcurrentMap<String, Boolean> patchFiles = new ConcurrentHashMap<>();
@@ -711,7 +712,7 @@ private ConcurrentMap<String, Boolean> initializePatching(RubyContext context) {
                         patchesDirectory,
                         new SimpleFileVisitor<Path>() {
                             @Override
-                            public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException {
+                            public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) {
                                 String relativePath = patchesDirectory.relativize(path).toString();
                                 if (relativePath.endsWith(".rb")) {
                                     patchFiles.put(relativePath.substring(0, relativePath.length() - 3), false);

src/main/java/org/truffleruby/core/GCNodes.java

@@ -12,6 +12,7 @@
 import java.lang.management.GarbageCollectorMXBean;
 import java.lang.management.ManagementFactory;
 
+import org.truffleruby.SuppressFBWarnings;
 import org.truffleruby.builtins.CoreMethod;
 import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
 import org.truffleruby.builtins.CoreModule;
@@ -50,6 +51,7 @@ protected Object vmGCStart() {
     @Primitive(name = "gc_force")
     public static abstract class GCForce extends PrimitiveArrayArgumentsNode {
 
+        @SuppressFBWarnings("DLS")
         @TruffleBoundary
         @Specialization
         protected Object force() {

src/main/java/org/truffleruby/core/MarkingService.java

@@ -90,6 +90,8 @@ public void runAllMarkers() {
             ExtensionCallStack stack = markingService.getThreadLocalData().getExtensionCallStack();
             stack.push(stack.getBlock());
             try {
+                // TODO (eregon, 15 Sept 2020): there seems to be no synchronization here while walking the list of
+                // markingService, and concurrent mutations seem to be possible.
                 MarkerReference currentMarker = markingService.getFirst();
                 MarkerReference nextMarker;
                 while (currentMarker != null) {

src/main/java/org/truffleruby/core/ReferenceProcessingService.java

@@ -16,6 +16,7 @@
 
 import com.oracle.truffle.api.CompilerDirectives;
 import org.truffleruby.RubyContext;
+import org.truffleruby.SuppressFBWarnings;
 import org.truffleruby.core.thread.RubyThread;
 import org.truffleruby.core.thread.ThreadManager;
 import org.truffleruby.language.control.RaiseException;
@@ -164,15 +165,17 @@ protected void processReferenceQueue(Class<?> owner) {
 
         protected void createProcessingThread(Class<?> owner) {
             final ThreadManager threadManager = context.getThreadManager();
+            RubyThread newThread;
             synchronized (this) {
                 if (processingThread != null) {
                     return;
                 }
-                processingThread = threadManager.createBootThread(threadName());
+                newThread = threadManager.createBootThread(threadName());
+                processingThread = newThread;
             }
             final String sharingReason = "creating " + threadName() + " thread for " + owner.getSimpleName();
 
-            threadManager.initialize(processingThread, null, threadName(), sharingReason, () -> {
+            threadManager.initialize(newThread, null, threadName(), sharingReason, () -> {
                 while (true) {
                     final ProcessingReference<?> reference = (ProcessingReference<?>) threadManager
                             .runUntilResult(null, processingQueue::remove);
@@ -271,6 +274,7 @@ protected synchronized void add(R newRef) {
         first = newRef;
     }
 
+    @SuppressFBWarnings("IS")
     protected R getFirst() {
         return first;
     }