Summary: | simple reconcile starts problem finder - main thread waiting | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Dani Megert <daniel_megert> | ||||||
Component: | Core | Assignee: | Jerome Lanneluc <jerome_lanneluc> | ||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||
Severity: | critical | ||||||||
Priority: | P3 | CC: | eric_jodet | ||||||
Version: | 3.3 | Keywords: | performance | ||||||
Target Milestone: | 3.3 M7 | ||||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
Whiteboard: | |||||||||
Attachments: |
|
Description
Dani Megert
2007-03-26 04:18:01 EDT
It looks like the working copy is not consistent before you call reconcile. So it is expected that problems are being found since the problem requestor is active. The ast that is created is an internal compiler ast (that is needed to find problems). No DOM AST is created in this case. Or did I miss something ? Mmmh, when I debug I see that problems get reported even if problemRequestor.isActive() returns false as this is not tested here: // report problems if (this.problems != null && (((this.reconcileFlags & ICompilationUnit.FORCE_PROBLEM_DETECTION) != 0) || !wasConsistent)) { if (problemRequestor != null) { In addition to the above code not checking for isActive() there is a race condition here: the text reconciler (thread) kicks in, enables the problem requestor and then the main thread simply wants to reconcile the structure without doing any more work but at that time the problem requestor is set to active. In the early days (up to < 3.2) this did not happen because we wrapped all calls to reconcile inside a synchronized(ICompilationUnit) {} to prevent parallel reconciles but then we got advised by JDT Core to remove this because it was claimed that JDT Core prevents two reconcile to run in parallel (this was debated at the JDT Summit 2005 in Zurich). (In reply to comment #2) > Mmmh, when I debug I see that problems get reported even if > problemRequestor.isActive() returns false Now I'm confused. In the original comment, the problem requestor was active (since this.resolveBindings was true). > as this is not tested here: > // report problems > if (this.problems != null && (((this.reconcileFlags & > ICompilationUnit.FORCE_PROBLEM_DETECTION) != 0) || !wasConsistent)) { > if (problemRequestor != null) { Right this is a bug and this needs to be fixed. Thanks for pointing at this. > In addition to the above code not checking for isActive() there is a race > condition here: the text reconciler (thread) kicks in, enables the problem > requestor and then the main thread simply wants to reconcile the structure > without doing any more work but at that time the problem requestor is set to > active. In the early days (up to < 3.2) this did not happen because we wrapped > all calls to reconcile inside a synchronized(ICompilationUnit) {} to prevent > parallel reconciles but then we got advised by JDT Core to remove this because > it was claimed that JDT Core prevents two reconcile to run in parallel (this > was debated at the JDT Summit 2005 in Zurich). Actually, I think we claimed that 2 reconciles could run in parallel as they are thread-safe. I still claim this is true. The problem is that the problem requestor is not thread-safe. Your implementation of isActive() should return true if running in the reconciler thread and it should return false if running in the main thread. This is easily achieved using a ThreadLocal field on the problem requestor to return the isActive state. Each thread can then set the isActive state indenpendantly of the other threads. >Now I'm confused. In the original comment, the problem requestor was active >(since this.resolveBindings was true). Yes. This is another issue. I tried to point that out in comment 2 with the words "In addition to the above" but obviously that wasn't clear enough - sorry about that. OK, I can indeed easily fix on my side (did so for next I-build), but we weren't aware of this. What is the real use case for running two reconciles on the same CU with same working copy owner, especially when there can only be one problem requestor? Fixed the isActive() on our side for I20070327-0800. (In reply to comment #4) > What is the real use case for running two reconciles on > the same CU with same working copy owner, especially when there can only be one > problem requestor? In fact, we cannot prevent 2 reconciles (on the same working copy etc.) to run at the same time, since this would require to lock on the cu (as you used to do) and since there can be compilation participants, this calls for deadlocks (classic pattern where 2 locks are taken in different order). I will fix the problem with beginReporting() being called when the problem requestor is not active. k. We can mark this fixed then. Created attachment 62064 [details]
Proposed fix and regression test
Created attachment 62072 [details]
New proposed fix and regression test
First fix was wrong since it would create an AST when:
- the working copy was consistent,
- the problem requestor was active
- problem detection was not forced
- the ast level was JLS3
Second fix and test released for 3.3M7 in HEAD. Verified for 3.3 M7 using build I20070427-0010 |