regression : Jar analyzer doesn't analyze the jars it used to for 0.7 (Bugzilla #13364)

[vladak]

status CLOSED severity normal in component analyzer for 0.9
Reported in version unspecified on platform ANY/Generic
Assigned to: Lubos Kosco

On 2009-12-15 10:28:43 +0000, Lubos Kosco wrote:

Happens with 0.8.1

Dec 14, 2009 8:06:22 AM org.opensolaris.opengrok.index.IndexDatabase addFile
INFO: Skipped file '/blahblah/server/lib/catalina-storeconfig.jar' because the analyzer didn't understand it.
Dec 14, 2009 8:06:22 AM org.opensolaris.opengrok.index.IndexDatabase addFile
FINE: Exception from analyzer:
org.apache.bcel.classfile.ClassFormatException: Invalid signature: `java/lang/Object;Lorg/apache/catalina/storeconfig/StoreDescription;)V'
at org.apache.bcel.classfile.Utility.signatureToString(Utility.java:878)
at org.apache.bcel.classfile.Utility.methodSignatureArgumentTypes(Utility.java:593)
at org.opensolaris.opengrok.analysis.executables.JavaClassAnalyzer.getContent(JavaClassAnalyzer.java:242)
at org.opensolaris.opengrok.analysis.executables.JavaClassAnalyzer.analyze(JavaClassAnalyzer.java:100)
at org.opensolaris.opengrok.analysis.executables.JarAnalyzer.analyze(JarAnalyzer.java:98)
at org.opensolaris.opengrok.analysis.AnalyzerGuru.getDocument(AnalyzerGuru.java:250)
at org.opensolaris.opengrok.index.IndexDatabase.addFile(IndexDatabase.java:560)
at org.opensolaris.opengrok.index.IndexDatabase.indexDown(IndexDatabase.java:690)
at org.opensolaris.opengrok.index.IndexDatabase.indexDown(IndexDatabase.java:673)
at org.opensolaris.opengrok.index.IndexDatabase.indexDown(IndexDatabase.java:673)
at org.opensolaris.opengrok.index.IndexDatabase.indexDown(IndexDatabase.java:673)
at org.opensolaris.opengrok.index.IndexDatabase.indexDown(IndexDatabase.java:673)
at org.opensolaris.opengrok.index.IndexDatabase.update(IndexDatabase.java:327)
at org.opensolaris.opengrok.index.IndexDatabase$1.run(IndexDatabase.java:142)
at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
at java.util.concurrent.FutureTask$Sync.innerRun(Unknown Source)
at java.util.concurrent.FutureTask.run(Unknown Source)
at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(Unknown Source)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
at java.lang.Thread.run(Unknown Source)

the same file is correctly analyzed with 0.7
I think this might be a problem because of bcel jars with 1.6 ...

On 2009-12-15 10:33:42 +0000, Lubos Kosco wrote:

bcel not thread safe:
http://www.mail-archive.com/bcel-dev@jakarta.apache.org/msg01235.html

oh well, so jar analyzer has to be improved

On 2010-01-10 22:02:30 +0000, Lubos Kosco wrote:

another thing I noticed is, that I often got the error:
" Cannot get tokens from an empty list! "

so my changes regarding 2.4.1 lucene might have caused this too,
we need tests for this and improve
http://src.opensolaris.org/source/search?q=List2TokenStream&project=%2Fopengrok

On 2010-01-10 22:08:09 +0000, Lubos Kosco wrote:

I will update bcel as well as fix the analyzer

On 2010-01-13 21:49:57 +0000, Lubos Kosco wrote:

I seriously think this is a regression of
Bug # 67: Duplicate magics in the analyzers

bcel is not thread safe and we were supposed to have an analyzer per thread, which in this case is not done, because of fixes in # 67 ...
DEFAULT_INSTANCE seems to be the evil ...
and as I can see we use DEFAULT_INSTANCE for zip, jar and plain analyzer
all of them need to be thread safe because of that
the confusion might be cause by confusing threadlocal cache when using zipanalyzer, which can return defaultinstance of jar analyzer, hence the proper factory model is broken

kah, can you help ?

On 2010-01-14 09:36:01 +0000, Knut Anders Hatlen wrote:

Bug # 67 has been fixed since 0.6.1, so I don't see how it could have caused a regression from 0.7 to 0.8. Also note that we only had one instance of JarAnalyzerFactory before that fix too. The fix just added a field to make it easier to get hold of the instance, as far as I can see.

The cached analyzers (the actual analyzers, not the analyzer factories) are not supposed to be shared between threads, by the way. They are stored in a ThreadLocal in the factory. See FileAnalyzerFactory.getAnalyzer().

On 2010-01-14 09:43:29 +0000, Lubos Kosco wrote:

(In reply to comment # 5)

Bug # 67 has been fixed since 0.6.1, so I don't see how it could have caused a
regression from 0.7 to 0.8. Also note that we only had one instance of
JarAnalyzerFactory before that fix too. The fix just added a field to make it
easier to get hold of the instance, as far as I can see.

The cached analyzers (the actual analyzers, not the analyzer factories) are not
supposed to be shared between threads, by the way. They are stored in a
ThreadLocal in the factory. See FileAnalyzerFactory.getAnalyzer().

I understand all above, I am just questioning it
because I cannot explain how is that we get this errors and we get " Cannot get tokens from an empty list! "
coming from jar analyzer ... (if we got 6 threads we only get this error message from one, but not from the rest of threads, which fail and leave dirty files behind ... )

it doesn't happen for any other analyzer it seems (so ThreadLocal kicks in there properly), so only this analyzer is hit and we didn't see this as a problem until we recently increased default number of threads ...

On 2010-01-14 09:44:30 +0000, Lubos Kosco wrote:

(In reply to comment # 6)

(In reply to comment # 5)

Bug # 67 has been fixed since 0.6.1, so I don't see how it could have caused a
regression from 0.7 to 0.8. Also note that we only had one instance of
JarAnalyzerFactory before that fix too. The fix just added a field to make it
easier to get hold of the instance, as far as I can see.

The cached analyzers (the actual analyzers, not the analyzer factories) are not
supposed to be shared between threads, by the way. They are stored in a
ThreadLocal in the factory. See FileAnalyzerFactory.getAnalyzer().

I understand all above, I am just questioning it
because I cannot explain how is that we get this errors and we get " Cannot get
tokens from an empty list! "
coming from jar analyzer ... (if we got 6 threads we only get this error
message from one, but not from the rest of threads, which fail and leave dirty
files behind ... )

sorry I mean we get the error from 5 analyzers which crash and only one goes through ...

it doesn't happen for any other analyzer it seems (so ThreadLocal kicks in
there properly), so only this analyzer is hit and we didn't see this as a
problem until we recently increased default number of threads ...

On 2010-01-14 11:25:51 +0000, Knut Anders Hatlen wrote:

Lubos, I filed a new bug for the List2TokenStream problem in case it's unrelated. See bug # 13884.

The first revision that fails with a ClassFormatException, is:

changeset: 508:7f3cfee18fc0
user: Trond Norbye trond.norbye@sun.com
date: Sun Aug 31 21:29:03 2008 +0200
summary: Added logging of exceptions instead of silently ignoring them

The problem was probably there before too, but we just ignored the exception.

On 2010-01-14 12:05:55 +0000, Knut Anders Hatlen wrote:

Multi-threaded indexing was not possible at the time bug # 67 was fixed, so the thread-safety problem could not be seen at that time. I updated my source tree to the revision where multi-threaded indexing was implemented (261:fd6b4fa4f533) and added print statements to show if ClassFormatExceptions were thrown. I did see the problem there as well (only that the indexer ignored the exceptions).

Then I backed out changeset 257:6fe5bd23fcb9 (fix for bug # 67), and I still saw the exceptions. So I'd conclude that these two bugs are not related.

On 2010-01-14 12:55:10 +0000, Lubos Kosco wrote:

fixed by just updating the bcel to 5.2

fixed in changeset: 929:576073c1ca02
OSR number for bcel is : 13368