Discussion:
[groovy-dev] groovy-core findbugs warnings
Pascal Schumacher
2015-02-22 13:24:27 UTC
Permalink
Hi all,

I added findbugs to the groovy build and the generated report contains
some warnings that seem worth looking at, e.g.:

* Sequence of calls to java.util.concurrent.ConcurrentHashMap may not
be atomic in
org.codehaus.groovy.runtime.ConversionHandler.invoke(Object, Method,
Object[])
* Inconsistent synchronization of
org.codehaus.groovy.runtime.metaclass.ConcurrentReaderHashMap.table;
locked 69% of time
* Inconsistent synchronization of
org.codehaus.groovy.runtime.metaclass.ThreadManagedMetaBeanProperty.initialValueCreator;
locked 66% of time
* Inconsistent synchronization of
org.codehaus.groovy.transform.tailrec.VariableExpressionReplacer.transformer;
locked 50% of time
* Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.classMetaClass in
org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.resetCachedMetaClasses()
* Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.instanceExclude
in
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getInstance(int)
* Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.instanceInclude
in
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getInstance(int)
* groovy.lang.ExpandoMetaClass.performOperationOnMetaClass(ExpandoMetaClass$Callable)
does not release lock on all exception paths

Maybe these are false positives, but it would be nice if somebody more
knowledgeable could take a look.

The full report with much more details (line numbers, etc.) can be
created by running "gradlew findbugsMain".

Regards,
Pascal
Pascal Schumacher
2015-02-22 13:29:23 UTC
Permalink
The report with these issues is also on the build server:

http://ci.groovy-lang.org/repository/download/Groovy_CoverageIndyRuntime/13866:id/target/reports/findbugs/main.html#Warnings_MT_CORRECTNESS
Post by Pascal Schumacher
Hi all,
I added findbugs to the groovy build and the generated report contains
* Sequence of calls to java.util.concurrent.ConcurrentHashMap may
not be atomic in
org.codehaus.groovy.runtime.ConversionHandler.invoke(Object,
Method, Object[])
* Inconsistent synchronization of
org.codehaus.groovy.runtime.metaclass.ConcurrentReaderHashMap.table;
locked 69% of time
* Inconsistent synchronization of
org.codehaus.groovy.runtime.metaclass.ThreadManagedMetaBeanProperty.initialValueCreator;
locked 66% of time
* Inconsistent synchronization of
org.codehaus.groovy.transform.tailrec.VariableExpressionReplacer.transformer;
locked 50% of time
* Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.classMetaClass in
org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.resetCachedMetaClasses()
* Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.instanceExclude
in
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getInstance(int)
* Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.instanceInclude
in
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getInstance(int)
* groovy.lang.ExpandoMetaClass.performOperationOnMetaClass(ExpandoMetaClass$Callable)
does not release lock on all exception paths
Maybe these are false positives, but it would be nice if somebody more
knowledgeable could take a look.
The full report with much more details (line numbers, etc.) can be
created by running "gradlew findbugsMain".
Regards,
Pascal
Guillaume Laforge
2015-02-23 14:09:35 UTC
Permalink
Some of these findings are indeed interesting and worth investigating!
Post by Pascal Schumacher
http://ci.groovy-lang.org/repository/download/Groovy_CoverageIndyRuntime/13866:id/target/reports/findbugs/main.html#Warnings_MT_CORRECTNESS
Hi all,
I added findbugs to the groovy build and the generated report contains
- Sequence of calls to java.util.concurrent.ConcurrentHashMap may not
be atomic in org.codehaus.groovy.runtime.ConversionHandler.invoke(Object,
Method, Object[])
- Inconsistent synchronization of
org.codehaus.groovy.runtime.metaclass.ConcurrentReaderHashMap.table; locked
69% of time
- Inconsistent synchronization of
org.codehaus.groovy.runtime.metaclass.ThreadManagedMetaBeanProperty.initialValueCreator;
locked 66% of time
- Inconsistent synchronization of
org.codehaus.groovy.transform.tailrec.VariableExpressionReplacer.transformer;
locked 50% of time
- Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.classMetaClass in
org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.resetCachedMetaClasses()
- Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.instanceExclude
in
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getInstance(int)
- Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.instanceInclude
in
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getInstance(int)
- groovy.lang.ExpandoMetaClass.performOperationOnMetaClass(ExpandoMetaClass$Callable)
does not release lock on all exception paths
Maybe these are false positives, but it would be nice if somebody more
knowledgeable could take a look.
The full report with much more details (line numbers, etc.) can be
created by running "gradlew findbugsMain".
Regards,
Pascal
--
Guillaume Laforge
Groovy Project Manager
Pivotal, Inc.

Blog: http://glaforge.appspot.com/
Social: @glaforge <http://twitter.com/glaforge> / Google+
<https://plus.google.com/u/0/114130972232398734985/posts>
Jochen Theodorou
2015-02-23 14:44:24 UTC
Permalink
Am 23.02.2015 15:09, schrieb Guillaume Laforge:
[...]
Post by Pascal Schumacher
* Sequence of calls to java.util.concurrent.ConcurrentHashMap
may not be atomic in
org.codehaus.groovy.runtime.ConversionHandler.invoke(Object,
Method, Object[])
this one is actually a false positive. Yes, it can happan that we put a
handle in the cache after there has been one added already. But since
both of them will be functionally equal, there is no big harm in a
replacement.

bye blackdrag
--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email
Jochen Theodorou
2015-02-23 14:55:05 UTC
Permalink
Post by Pascal Schumacher
Inconsistent synchronization of
org.codehaus.groovy.runtime.metaclass.ConcurrentReaderHashMap.table;
locked 69% of time
This one can be crossed off too. As I did learn the message is about
complaining about not all reads being synchronized the same way as the
writes. Again this is on purpose here.

bye blackdrag
--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email
Jochen Theodorou
2015-02-23 14:58:53 UTC
Permalink
Post by Pascal Schumacher
Inconsistent synchronization of
org.codehaus.groovy.runtime.metaclass.ThreadManagedMetaBeanProperty.initialValueCreator;
locked 66% of time
This one could be more serious, but I think we should deprecate
everything that is related to that field. It seems not to be really used.

bye blacdrag
--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email
Jochen Theodorou
2015-02-23 15:01:01 UTC
Permalink
Post by Pascal Schumacher
Inconsistent synchronization of
org.codehaus.groovy.transform.tailrec.VariableExpressionReplacer.transformer;
locked 50% of time
Here I would say replaceExpressionPropertyWhenNecessary misses a
synchronization block for the read of transformer
--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email
Jochen Theodorou
2015-02-23 15:10:14 UTC
Permalink
Post by Pascal Schumacher
Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.classMetaClass in
org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.resetCachedMetaClasses()
This one Cedric and me had a lengthy discussion about in the past already...
--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email
Jochen Theodorou
2015-02-23 15:13:01 UTC
Permalink
# Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.instanceExclude
in
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getInstance(int)
# Incorrect lazy initialization of static field
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.instanceInclude
in
org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl.getInstance(int)
Here I think findbugs may have found something... the fields are
potentially under concurrent access, but not treated like that. Normally
in groovy there are no includes and excludes, so the code is not
executed. But I guess this is a bug.
--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email
Jochen Theodorou
2015-02-23 15:16:05 UTC
Permalink
Post by Pascal Schumacher
groovy.lang.ExpandoMetaClass.performOperationOnMetaClass(ExpandoMetaClass$Callable)
does not release lock on all exception paths
This one I am not sure... must oversee an exception path
--
Jochen "blackdrag" Theodorou - Groovy Project Tech Lead
blog: http://blackdragsview.blogspot.com/
german groovy discussion newsgroup: de.comp.lang.misc
For Groovy programming sources visit http://groovy-lang.org


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email
Loading...