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

Bug 315845

Summary: Intermittent race-condition failure in MIMemoryTest.asynchronousReadWrite()
Product: [Tools] CDT Reporter: John Cortell <john.cortell>
Component: cdt-debug-dsf-gdbAssignee: Project Inbox <cdt-debug-dsf-gdb-inbox>
Status: RESOLVED FIXED QA Contact: Marc Khouzam <marc.khouzam>
Severity: normal    
Priority: P3 CC: pawel.1.piech
Version: 7.0Flags: john.cortell: review?
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Fix john.cortell: iplog-

Description John Cortell CLA 2010-06-04 17:41:42 EDT 
I normally wouldn't create a bugzilla for this type of issue, but I need a placeholder for this fix until we unfreeze HEAD post Helios.

An explanation of the issue is in the code (patch).
Comment 1 John Cortell CLA 2010-06-04 17:43:58 EDT 
Created attachment 171182 [details]
Fix

Fix
Comment 2 John Cortell CLA 2010-06-04 17:45:16 EDT 
Marc, please review.
Comment 3 Marc Khouzam CLA 2010-06-04 20:54:34 EDT 
Wow, nice find.  

I think this is a lacking in the AsyncCompletionWaitor.  The CountingRequestMonitor had a similar issue.  Pawel fixed it by basically not allowing the CRM to complete until it was told how many results it was expecting in total.  Maybe we should do the same here and instead of having increment() called for every attempt, one single setCount() should be called once we know how many attempts.

But for the short-term, I'm fine with this fix.
Comment 4 Marc Khouzam CLA 2010-06-04 20:55:43 EDT 
Did you really want to mark this as fixed already?
Comment 5 John Cortell CLA 2010-06-07 11:28:51 EDT 
(In reply to comment #3)
> Wow, nice find.  
> 
> I think this is a lacking in the AsyncCompletionWaitor.  The
> CountingRequestMonitor had a similar issue.  Pawel fixed it by basically not
> allowing the CRM to complete until it was told how many results it was
> expecting in total.  Maybe we should do the same here and instead of having
> increment() called for every attempt, one single setCount() should be called
> once we know how many attempts.
> 
> But for the short-term, I'm fine with this fix.

Well, I think a variant of increment that takes an int would be good. I think we want to keep the simple increment() method, since it's useful in some cases. Adding a setCount() would be confusing, IMO.

Reopening. Will move to fixed when committed (after Helios).
Comment 7 CDT Genie CLA 2010-06-15 18:23:01 EDT 
*** cdt cvs genie on behalf of jcortell ***
Bug 315845: Intermittent race-condition failure in MIMemoryTest.asynchronousReadWrite()

[*] AsyncCompletionWaitor.java 1.4.2.1 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/AsyncCompletionWaitor.java?root=Tools_Project&r1=1.4&r2=1.4.2.1
Comment 8 John Cortell CLA 2010-06-15 18:31:55 EDT 
Committed to HEAD and cdt_7_0