emopers

Hi, we have an improvement to this property in order to reduce the false positives it could trigger. 
This problem was found when we submitted the bug reports for this project (link: https://github.com/threerings/playn/pull/82/files). 
The developer rejected our PR as `bytes`(link: https://github.com/threerings/playn/blob/98c012474b82e4cf6cb56f9ffd837f8777ab8eea/core/src/playn/core/json/JsonAppendableWriter.java#L35)
here was actually flushed after it's been casted into Flushable.
This is because JavaMOP only catches OutputStream object, while missing on the superclass Flushable of OutputStream. 
We'd suggest to improve this property by also considering the supertype of the monitored object to avoid false positives triggered by type casting.
Furthermore, though this is the improvement of this specific property since we found false positives associated with it,
the formation of other properties could also consider the supertype to reduce potential false positives.

emopers

We have seen large amount of violations for
{Boolean, Byte, Character, Double, Float, Long, Integer, Short}_StaticFactory properties during runtime monitoring.
However, we don't feel these properties very useful in monitoring most appplication programs. 
Though it is stated in Java API (link: http://docs.oracle.com/javase/6/docs/api/java/lang/Integer.html#valueOf%28int%29)that using StaticFactory 
"is likely to yield significantly better space and time performance by caching frequently requested values.", 
we think that 1) these performance bugs are less concerned for common developers than functional bugs and
2) according to our experiment, using static factory seems not render better space and time performance, or even worse.
Following is the snippet we used for experiment:
import java.util.Random;
 2

emopers

In RealTimePlayer.java, Math.random() is used for random number generation. 
According to Java API (Link: http://docs.oracle.com/javase/8/docs/api/java/lang/Math.html#random--),
it's suggested that each thread has their own random number generator to reduce contention.
The increasing in contention might have a significant effect since the random number generation is 
in Thread implementation. 
This pull request improved the condition by replacing Math.random() with Random.nextInt().

emopers

In TraceCreator.java, Math.random() is used for random number generation. 
According to Java API (Link: http://docs.oracle.com/javase/8/docs/api/java/lang/Math.html#random--),
it's suggested that each thread has their own random number generator to reduce contention.
This pull request improved the condition by replacing Math.random() with Random.nextInt().

emopers

##Unneccessary property##
Long_BadParsingArgs property (http://goo.gl/F6ygPN) would trigger violations if a String passed into Long.parseLong() 
is not valid for representing a Long value. JVM will throw NumberFormatException upon receiving an invalid input, 
which is exactly the same thing as what this property does. This property does not provide more useful debugging 
information than JVM exception itself, nor does it provide insight into the prevention of such Exception. For all the
violations we got from monitoring with this property on 200 open source projects with passing tests are false alarm, 
for which the Exceptions thrown by JVM are used to recognize the validity of inputs and well handled. 

emopers

##Unncessary property##
Object_MonitorOwner (https://goo.gl/5Ek4FX) requires a Thread to be the owner of an Object before calling notify() or wait() 
on this Object. This property would trigger violations by detecting if Threads.holdsLock(o) is true when calling notify(o) 
wait(o), which is the condition that JVM will throw an IllegalMonitorStateException. The violations triggered by this property
do not provide more useful debugging information than the exception from JVM, nor do they provide any insight into
the prevention of the exception. We'd suggest to remove this property.
The snippet below presents the condition described by this property that JavaMOP would trigger a violation. 
Compile and execute the program below with JDK 1.6-1.8, IllegalMonitorStateException would be thrown at "o.notify();", 
where current thread is not Thread t which holds Object o. 

emopers

##Improvement in property##
Iterator_hasNext property (https://goo.gl/sO43It) requires a program to guarantee that to call Iterator.hasNext() 
before calling Iterator.next() in order to prevent the NullPointerException triggered when there is no next element in 
the underlying collection without proper debugging information. From our inspection of the monitoring
on 200 open source projects with this property, we found that most false alarms of this property was
triggered because the developer check the validity of calling Iterator.next() by means beside calling Iterator.hasNext(),
typically by calling collection.isEmpty() or collection.size(). We hereby suggest adding such checks as valid
prerequisites for calling Iterator.next() to avoid false alarms.
For example, in compile-testing 
https://github.com/google/compile-testing/blob/51cdab8998ea25210e8d3726982947054f421f94/src/main/java/com/google/testing/compile/TypeEnumerator.java#L102

emopers

Hi, we're rethinking about the usability of the property Comparable_CompareToNull in finding bugs. 
As stated in Java API doc, "e.compareTo(null) should throw a NullPointerException even though e.equals(null) returns false.",
so intuitively this property would be violated when and only when NullPointerException is thrown by JVM given
how this property is written. Though there is possibility that some implementations of Comparable interface may not throw NullPointerException as
required by Java specification, this could be detected by property Comparable_CompareToNullException. 

In our samples of this property's violations from monitoring it on 200 open source projects, 
all violations of this kind are triggered by developers' intentionally passing null in negative tests, 
which conforms with the argument above. Furthermore, violations of this property do not provide more useful debugging information than 
the NullPointerException itself.  

emopers

Title: 
NNStorage does not synchronize iteration on a synchronized list
Body:
In line 839 of NNStroage.java#reportErrorsOnDirectories, the synchronized list, `sds`
is iterated in an unsynchronized manner, but according to [Oracle Java 7 API specification]
(http://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#synchronizedList(java.util.List)),
this is not thread-safe and can lead to non-deterministic behavior.
This pull request adds a fix by synchronizing the iteration on `sds`. The synchronized list is passed to method `reportErrorsnODirectories` from 
[here](https://github.com/facebookarchive/hadoop-20/blob/2a29bc6ecf30edb1ad8dbde32aa49a317b4d44f4/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSImage.java#L508)

emopers

Title:
Closing DataOutputStream before calling toByteArray on the underlying ByteArrayOutputStream
Subject:
 When an DataOutputStream instance wraps an underlying ByteArrayOutputStream instance,
  it is recommended to flush or close the ObjectOutputStream before invoking the underlying instances's toByteArray().
Although in these cases this is not strictly necessary because the
DataOutputStream flush and close method have no effect. However, it is a good practice to call
flush/close explicitly as mentioned for example (here)
[http://stackoverflow.com/questions/2984538/how-to-use-bytearrayoutputstream-and-dataoutputstream-simultaneously-java]
This pull request adds close methods to close DataOutputStream. Even in the cases where DataOutputStream can