[REEF-768] Fix high-priority warnings reported by Findbugs

• Type: Bug
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Fix Version/s: 0.13
• Assignee: Mariia Mykhailova
• Reporter: Mariia Mykhailova
• Created: 17/Sep/15 23:16
• Updated: 22/Sep/15 22:29
• Resolved: 22/Sep/15 00:20

Description

• There is an apparent infinite recursive loop in org.apache.reef.wake.impl.OpaqueLocalIdentifier.toString()

    public String toString() {
      return "local-opaque-id://" + this.toString();
    }
  

  I'm puzzled as to what was supposed to be here - maybe some kind of field which actually stored id?

• Bad comparison of nonnegative value with -1 in org.apache.reef.wake.profiler.WakeProfiler.newSetVertex(Set) (line 74)
  Should be replaced with 1, judging from the code

• Field only ever set to null: org.apache.reef.wake.storage.SequentialFileReader.dest
  Do we need to fix this, or maybe we should delete SequentialFileReader which we never use (and which would throw NullPointerException if ever used - so don't even need to go through deprecation)? If we choose to delete it, there is a bunch of related classes under org.apache.reef.wake.storage which are used only in this reader, so they might be deprecation material too

• Boxing/unboxing to parse a primitive ...
  We're using Integer.valueOf (which returns Integer), should use Integer.parseInt (which returns int).

• USELESS_STRING In method org.apache.reef.javabridge.generic.JobDriver$ContextMessageHandler.onNext(ContextMessage)
  message.get() is a byte[]; calling toString() on it produces something like [C@16f0472. Should use new String(message.get()) instead.

• org.apache.reef.javabridge.NativeInterop.HANDLERS is a mutable collection
  Findbugs suggestion is to wrap the map in Collections.unmodifiableMap - no harm should be done

• org.apache.reef.runtime.common.evaluator.context.ContextManager.removeContext(String) forces garbage collection; extremely dubious except in benchmarking code
  The code has a `// TODO sure??` comment. We should either fix the comment to clarify our intention or remove the line

• Possible null pointer dereference of HttpServerShellCmdtHandler.cmdOutput in org.apache.reef.examples.hellohttp.HttpServerShellCmdtHandler.onHttpRequest(ParsedHttpRequest, HttpServletResponse)
  Indeed it looks like cmdOutput can be null after the loop; should we wrap writing to output stream in if (cmdOutput != null)?

• Useless condition: it's known that this.failDriverOnEvaluatorLogErrors == true at this point
  In method org.apache.reef.runtime.yarn.driver.restart.DFSEvaluatorPreserver.handleException(Exception, String, String)
  Indeed, the method can be simplified

• There is also a number of internationalization warnings
  DM_DEFAULT_ENCODING: Reliance on default encoding
  Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
  Should set a specific encoding, probably UTF-8.