Bug 320809 - ArrayIndexOutOfBoundsException in IndexManager.writeSavedIndexNamesFile - concurrency issue?
Summary: ArrayIndexOutOfBoundsException in IndexManager.writeSavedIndexNamesFile - con...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows Vista
: P3 minor (vote)
Target Milestone: 3.7 M2   Edit
Assignee: Satyam Kandula CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 215033 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-07-24 09:34 EDT by Richard Melvin CLA
Modified: 2019-12-29 07:59 EST (History)
4 users (show)

See Also:

Attachments
error details (5.67 KB, text/plain)
2010-07-24 09:38 EDT, Richard Melvin CLA 		no flags Details
Proposed patch (1.33 KB, patch)
2010-07-28 02:23 EDT, Satyam Kandula CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Melvin CLA 2010-07-24 09:34:43 EDT 
Build Identifier: 20100617-1415

Error log reports a mess of errors on opening one project, with the first one being 

java.lang.ArrayIndexOutOfBoundsException: 42
at org.eclipse.jdt.internal.core.search.indexing.IndexManager.writeSavedIndexNamesFile(IndexManager.java:955)
at org.eclipse.jdt.internal.core.search.indexing.IndexManager.updateIndexState(IndexManager.java:894)
at org.eclipse.jdt.internal.core.search.indexing.IndexManager.rebuildIndex(IndexManager.java:553)
at org.eclipse.jdt.internal.core.search.indexing.IndexManager.getIndex(IndexManager.java:237)
at org.eclipse.jdt.internal.core.search.indexing.IndexManager.getIndexes(IndexManager.java:312)
at org.eclipse.jdt.internal.core.search.PatternSearchJob.getIndexes(PatternSearchJob.java:81)
at org.eclipse.jdt.internal.core.search.PatternSearchJob.ensureReadyToRun(PatternSearchJob.java:50)
at org.eclipse.jdt.internal.core.search.processing.JobManager.performConcurrentJob(JobManager.java:174)

Reproducible: Couldn't Reproduce

Steps to Reproduce:
n/a
Comment 1 Richard Melvin CLA 2010-07-24 09:38:36 EDT 
Created attachment 175151 [details]
error details

error details from exception, with source code deleted
Comment 2 Richard Melvin CLA 2010-07-24 09:51:57 EDT 
Checking the source code for IndexManager, it looks to be some kind of concurrency issue.

1. writeSavedIndexNamesFile traverses both indexStates.keyTable and indexStates.valueTable using a length taken from only one of them. So any concurrent modification of indexStates can get an exception at lines 955 (if the update happens between line 952 and 953).

2. writeSavedIndexNamesFile has all paths to it synchronised.

3. getIndexStates is unsynchronised, and called from public function that are a mixture of sync and unsync. In particular, computeIndex and ensureIndexExists appear to be called from other threads.
Comment 3 Satyam Kandula CLA 2010-07-26 02:52:01 EDT 
(In reply to comment #2)
Thanks for the analysis. getIndexStates() could potentially create a problem only in the beginning of the execution. It does put in the table only for the first time and from the next time onwards, it just returns. 
Did you run into the problem at the startup?
Comment 4 Richard Melvin CLA 2010-07-26 18:43:43 EDT 
(In reply to comment #3)

Yes, the error occurred on startup.
Comment 5 Satyam Kandula CLA 2010-07-28 02:20:47 EDT 
This is what I think is happening. A call to getIndexStates() from computeIndexLocation() or ensureIndexExists() is trying to build the indexStates. The subsequent call through some other synchronized function thinks the indexStates is initialized and starts using the table and running into some inconsistency during the initialization. 
Though we could try to synchronize just the block of indexStates initialization, we could end up with similar problem in computeIndexLocation() or ensureIndexExists(). Hence, I think the best way to fix this could be make both these functions synchronized.
Comment 6 Satyam Kandula CLA 2010-07-28 02:23:07 EDT 
Created attachment 175368 [details]
Proposed patch

Made both computeIndexLocation() and ensureIndexExists() synchronized.  I don't have any test for this. 
Srikanth, Could you please review? TIA.
Comment 7 Satyam Kandula CLA 2010-07-28 02:26:21 EDT 
Srikanth, Can you please review this patch? The patch is very small and doesn't have any test. TIA.
Comment 8 Srikanth Sankaran CLA 2010-08-02 00:03:08 EDT 
(In reply to comment #7)
> Srikanth, Can you please review this patch? The patch is very small and doesn't
> have any test. TIA.

Patch looks good.
Comment 9 Satyam Kandula CLA 2010-08-09 01:51:28 EDT 
Released on HEAD.
Comment 10 Ayushman Jain CLA 2010-09-13 07:31:22 EDT 
Verified for 3.7M2 using code inspection
Comment 11 Stephan Herrmann CLA 2019-12-29 07:59:44 EST 
*** Bug 215033 has been marked as a duplicate of this bug. ***