Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 312334

Summary: Implicit Jobs (ThreadJobs) in some circumstances are automatically run twice. Resuming from a yieldRule() cause jobs to be rescheduled. Job.schedule() should have no effect for ThreadJob. Job.yieldRule() for ThreadJob does not find blocked explicit Jobs.
Product: [Eclipse Project] Platform Reporter: Min Idzelis <min123>
Component: RuntimeAssignee: John Arthorne <john.arthorne>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: john.arthorne, tjwatson
Version: 3.6Flags: john.arthorne: review+
tjwatson: review+
dj.houghton: review+
Target Milestone: 3.6 RC2   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Fix v1
john.arthorne: iplog+
Regression Junits
john.arthorne: iplog+
Updated tests none

Description Min Idzelis CLA 2010-05-10 16:31:35 EDT 
Build Identifier: 

This is a combination of 3 very closely related bugs, which is why they are being reported together. 

It all stems from the same root cause, and this cause let me to uncover additional bugs that were always present in the system. 

The root cause exists when a Job that has a non-null Rule is running, and a beginRule() is performed with a nested non-null Rule. 

In these circumstances, the active implicit job (ThreadJob) will choose another ThreadJob to unblock. This usually works. However,the problem is that there are 2 ThreadJobs that represent the same real job. 

There are several causes to why there are two ThreadJobs for the same actual real Job. 
1) In resuming from a yield, the startTime of the job is not changed back to T_NONE. This causes the job to be rescheduled() when it is ended. 
2) In starting a nested ThreadJob via beginRule(), the code path that changes state of this ThreadJob is not invoked. When this inner ThreadJob is ended, it is rescheduled. 
3) If schedule() is called on JobManager.currentJob() after beginRule(). 

Additionally, I noticed that yieldRule() does not look for any blocked jobs on any implicit job. This is a flaw with yieldRule() as it was intended that yielding an implicit job should allow blocked jobs to proceed. 

I have regression test cases and a fix ready. Please stay tuned for the forthcoming patches. 

Reproducible: Always
Comment 1 Min Idzelis CLA 2010-05-10 18:21:10 EDT 
Created attachment 167837 [details]
Fix v1

Fixes
Comment 2 Min Idzelis CLA 2010-05-10 18:21:38 EDT 
Created attachment 167838 [details]
Regression Junits
Comment 3 John Arthorne CLA 2010-05-12 18:05:18 EDT 
I agree the fix is needed and this fix looks good. In particular I like that the fix only touches the yield implementation which is new this release, so doesn't risk wider regressions. The only exception is the fix for 3), which involves adding a shouldRun method to ThreadJob. This is an existing bug and it seems someone calling currentJob().schedule() is unlikely, but I agree we should fix it. Having multiple ThreadJobs for a given thread at once is quite likely to cause other bugs outside the yield implementation.

I'd like to put this in for RC1 but I'm afraid I have left this too late and won't be able to find another reviewer in time. If not I will track down reviewers and make sure it gets into RC2. Tom, I can talk to you about this tomorrow.
Comment 4 John Arthorne CLA 2010-05-13 16:30:50 EDT 
Created attachment 168468 [details]
Updated tests

Updated tests to avoid side-effects that cause other tests to fail.
Comment 5 John Arthorne CLA 2010-05-15 21:21:28 EDT 
Fix and tests released. Thanks all.