Bug 327143

Summary: IndexManager should not accept new jobs if the processing thread is null
Product: [Eclipse Project] JDT Reporter: Olivier Thomann <Olivier_Thomann>
Component: CoreAssignee: Satyam Kandula <satyam.kandula>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 Flags: Olivier_Thomann: review+
Version: 3.7   
Target Milestone: 3.7 M5   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Proposed patch
satyam.kandula: review?
Proposed fix none

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.