Bug 327143 - IndexManager should not accept new jobs if the processing thread is null
Summary: IndexManager should not accept new jobs if the processing thread is null
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.7 M5   Edit
Assignee: Satyam Kandula CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-06 13:37 EDT by Olivier Thomann CLA
Modified: 2011-02-01 09:59 EST (History)
0 users

See Also:
Olivier_Thomann: review+


Attachments
Proposed patch (1.07 KB, patch)
2010-12-16 06:26 EST, Satyam Kandula CLA
satyam.kandula: review?
Details | Diff
Proposed fix (1.55 KB, patch)
2011-01-25 09:05 EST, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Thomann CLA 2010-10-06 13:37:20 EDT
I ran into a case where a simulateExit() was called and the next touch(..) call failed. The consequence was that the simulateRestart() was never called.
But a new jar file added ended up being added into the waiting jobs and the index manager was in an infinite loop and the number of awaiting jobs was always 1 and since the processing thread was null, that job was never run.

So we must make sure that the index manager doesn't accept new jobs if the processing thread is null or if stops the loop when there is an awaiting job but the thread is null.
Right now the loop never ends unless the number of awaiting jobs is 0.
Comment 1 Satyam Kandula CLA 2010-12-16 06:26:47 EST
Created attachment 185305 [details]
Proposed patch

Olivier, Please look at the patch and let me know if you are mentioning about the same thing and if so, please review!
Comment 2 Olivier Thomann CLA 2010-12-16 09:12:36 EST
This would throw a OperationCanceledException even if the operation was not really canceled.
Why not simply adding: && this.processingThread != null inside the while condition ?
Since the processingThread is nullify on shutdown, I don't think this is a big difference.
Satyam, +1 for this and you can decide how you want to fix it.
Comment 3 Satyam Kandula CLA 2010-12-16 09:21:21 EST
(In reply to comment #2)
> This would throw a OperationCanceledException even if the operation was not
> really canceled.
> Why not simply adding: && this.processingThread != null inside the while
> condition ?
> Since the processingThread is nullify on shutdown, I don't think this is a big
> difference.
> Satyam, +1 for this and you can decide how you want to fix it.
This is to ensure that client gets to know that the operation didn't complete properly for whatever reason.
Comment 4 Olivier Thomann CLA 2010-12-16 09:30:59 EST
(In reply to comment #3)
> This is to ensure that client gets to know that the operation didn't complete
> properly for whatever reason.
Fine for me then.
Comment 5 Satyam Kandula CLA 2010-12-20 05:25:02 EST
Released in HEAD
Comment 6 Olivier Thomann CLA 2011-01-25 09:05:44 EST
Created attachment 187517 [details]
Proposed fix

I wonder if it is not better to fix it that way. When no progress monitor is given, you can still end up with a hang.
Comment 7 Olivier Thomann CLA 2011-01-25 10:40:25 EST
This will be verified once a build with the last fix is available.
Comment 8 Olivier Thomann CLA 2011-02-01 09:59:34 EST
Verified.