Bug 312326 - IllegalArgumentException using open type dialog
Summary: IllegalArgumentException using open type dialog
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 3.6 RC1   Edit
Assignee: Satyam Kandula CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 319832 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-05-10 15:48 EDT by Darin Wright CLA
Modified: 2010-07-28 02:35 EDT (History)
6 users (show)

See Also:
frederic_fusier: review+
darin.eclipse: review+


Attachments
Proposed patch (2.19 KB, patch)
2010-05-12 05:04 EDT, Satyam Kandula CLA
frederic_fusier: review+
Details | Diff
New proposal (1.67 KB, patch)
2010-05-12 13:54 EDT, Olivier Thomann CLA
Olivier_Thomann: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Wright CLA 2010-05-10 15:48:00 EDT
Using the open type dialog, I'm seeing a lot of the following. Errors, not sure if the problem comes from UI or Core.

eclipse.buildId=I20100506-0800
java.version=1.6.0_18
java.vendor=Sun Microsystems Inc.
BootLoader constants: OS=win32, ARCH=x86, WS=win32, NL=en_US
Command-line arguments:  -os win32 -ws win32 -arch x86


Error
Mon May 10 14:46:27 CDT 2010
An internal error occurred during: "Items filtering".

java.lang.IllegalArgumentException: BundleValidationOperation.java is not a valid class file name
	at org.eclipse.jdt.internal.core.PackageFragment.getClassFile(PackageFragment.java:182)
	at org.eclipse.jdt.internal.core.search.TypeNameMatchRequestorWrapper.createTypeFromJar(TypeNameMatchRequestorWrapper.java:173)
	at org.eclipse.jdt.internal.core.search.TypeNameMatchRequestorWrapper.acceptType(TypeNameMatchRequestorWrapper.java:113)
	at org.eclipse.jdt.internal.core.search.BasicSearchEngine$3.acceptIndexMatch(BasicSearchEngine.java:1124)
	at org.eclipse.jdt.core.search.SearchPattern.acceptMatch(SearchPattern.java:309)
	at org.eclipse.jdt.core.search.SearchPattern.findIndexMatches(SearchPattern.java:2317)
	at org.eclipse.jdt.internal.core.search.matching.MatchLocator.findIndexMatches(MatchLocator.java:264)
	at org.eclipse.jdt.internal.core.search.PatternSearchJob.search(PatternSearchJob.java:97)
	at org.eclipse.jdt.internal.core.search.PatternSearchJob.execute(PatternSearchJob.java:63)
	at org.eclipse.jdt.internal.core.search.processing.JobManager.performConcurrentJob(JobManager.java:276)
	at org.eclipse.jdt.internal.core.search.BasicSearchEngine.searchAllTypeNames(BasicSearchEngine.java:1135)
	at org.eclipse.jdt.core.search.SearchEngine.searchAllTypeNames(SearchEngine.java:846)
	at org.eclipse.jdt.internal.ui.dialogs.FilteredTypesSelectionDialog.fillContentProvider(FilteredTypesSelectionDialog.java:557)
	at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$FilterJob.filterContent(FilteredItemsSelectionDialog.java:2188)
	at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$FilterJob.internalRun(FilteredItemsSelectionDialog.java:2130)
	at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$FilterJob.doRun(FilteredItemsSelectionDialog.java:2102)
	at org.eclipse.ui.dialogs.FilteredItemsSelectionDialog$FilterJob.run(FilteredItemsSelectionDialog.java:2089)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
Comment 1 Dani Megert CLA 2010-05-11 02:16:21 EDT
Satyam, can you take a look? It looks like bug 286379 is back.

Darin, are you 100% sure that you're on I20100506-0800? What's the version of the resolved jdt.core bundle?
Comment 2 Satyam Kandula CLA 2010-05-11 02:50:14 EDT
Darin,
Can you give the workspace that you were using? 

This scenario is little different from bug 286379. We were unable to reproduce this scenario. Having the workspace should help.
Comment 3 Darin Wright CLA 2010-05-11 09:39:26 EDT
(In reply to comment #1)
> Satyam, can you take a look? It looks like bug 286379 is back.
> Darin, are you 100% sure that you're on I20100506-0800? What's the version of
> the resolved jdt.core bundle?

jdt.core is "3.6.0.v_A50"
Comment 4 Satyam Kandula CLA 2010-05-11 14:28:58 EDT
Darin gave me the workspace - Thanks Darin for it. 

I couldn't reproduce the issue directly with the workspace but one index file
got corrupted. 2636936256.index should have been the index file for
jre\lib\ext\localedata.jar. However, the following java files had got inside
this index file causing this problem  
#####
src/org/eclipse/pde/internal/core/PluginModelManager.java
src/org/eclipse/pde/internal/core/BundleValidationOperation.java
src/org/eclipse/pde/internal/core/ExternalModelManager.java
src/org/eclipse/pde/internal/core/ErrorMessage.java
src/org/eclipse/pde/internal/core/ICoreConstants.java
src/org/eclipse/pde/internal/core/TargetPreferenceModifyListener.java
######
These java files are also in another index file 1094944507.index, where they
rightly belong. 

Need to find out why the java files could have got inside the index files meant
for another jar. 

OpenType should work after deleting
.metadata\.plugins\org.eclipse.jdt.core\2636936256.index. 
Darin, Please try to delete this file and then bring up eclipse and see if
OpenType could work.
Comment 5 Darin Wright CLA 2010-05-11 14:36:53 EDT
(In reply to comment #4)
> OpenType should work after deleting
> .metadata\.plugins\org.eclipse.jdt.core\2636936256.index. 
> Darin, Please try to delete this file and then bring up eclipse and see if
> OpenType could work.

This worked. I no longer see the exception.
Comment 6 Satyam Kandula CLA 2010-05-12 05:00:45 EDT
Very few java files ended up in the wrong index file. This could have been happened only if the wrong index file is used. The mapping is not stored but computed through the checksum. I believe IndexManager#computeIndexLocation() is returning the wrong index wrong times. I couldn't think of any other possibilites. This function is not thread safe and there are possibilities that multiple threads can get into this function at the same time. At the beginning of the execution, both the reconciler thread and the indexer thread enter this code. The reconciler thread gets in here to get the subtypes. 

This function is not thread safe because of the following reasons. 
1. CRC32 Object is not a local variable but a static field. I don't see the necessity of it. The constructor doesn't seem to be costly and the accesses are less. 
2. SimpleLookupTable#put() is also not thread safe.
Comment 7 Satyam Kandula CLA 2010-05-12 05:04:22 EDT
Created attachment 168101 [details]
Proposed patch

Patch to make IndexManager#computeIndexLocation() thread safe.
Comment 8 Satyam Kandula CLA 2010-05-12 05:07:30 EDT
Frederic,
What do you think of this possibility? Do you see any other possibilities? Can you review the patch accordingly?
Comment 9 Frederic Fusier CLA 2010-05-12 05:39:18 EDT
(In reply to comment #8)
> Frederic,
> What do you think of this possibility? Do you see any other possibilities? Can
> you review the patch accordingly?

Well spotted Satyam :-)

I do not see any other possibility but this one seems to be a real good explanation of this issue and should fix the numerous exception we see since a long time in this area!

The patch looks good to me. IMO, most of the time while using the CRC is spent during the update, not while creating the instance. Hence, I do not expect any performance impact with this fix.
Comment 10 Olivier Thomann CLA 2010-05-12 13:54:57 EDT
Created attachment 168214 [details]
New proposal

Same patch without the synchronized block.
The fix is really to use a local CRC32 to compute the checksum.
No need to add a synchronized block at the end.
Frédéric, please review.
Comment 11 Olivier Thomann CLA 2010-05-12 15:31:03 EDT
Darin, please review.
Comment 12 Olivier Thomann CLA 2010-05-12 15:31:29 EDT
Comment on attachment 168214 [details]
New proposal

+ for iplog since this comes from Satyam's patch.
Comment 13 Darin Wright CLA 2010-05-12 15:45:35 EDT
+1 Looks good.
Comment 14 Olivier Thomann CLA 2010-05-12 20:54:07 EDT
Released for 3.6RC1.
Comment 15 Jay Arthanareeswaran CLA 2010-05-17 04:09:31 EDT
Verified for 3.6RC1 by code inspection.
Comment 16 Satyam Kandula CLA 2010-07-28 02:35:38 EDT
*** Bug 319832 has been marked as a duplicate of this bug. ***