Closed Bug 578589 Opened 14 years ago Closed 14 years ago

Intermittent failure in toolkit/content/tests/widgets/test_toolbar.xul | | checkSet(76) correct ID /^spacer\d+/ for toolbar tb-test; got: p12 followed by two more checkSet(76) failures

Categories

(Toolkit :: UI Widgets, defect)

Product:
Toolkit
Component:
UI Widgets
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla2.0b7

People

(Reporter: ehsan.akhgari, Assigned: mak)

References

Details

(Keywords: intermittent-failure, Whiteboard: [time-bomb][relevant comments: 262,263,264,278,286])

Attachments

(1 file)

patch v1.0
1.18 KB, patch
enndeakin
: review+
beltzner
: approval2.0+
Details | Diff | Splinter Review 
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1279060632.1279061242.30409.gz
WINNT 5.2 mozilla-central opt test mochitests-5/5 on 2010/07/13 15:37:12

12786 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_toolbar.xul | checkSet(76) correct ID /^spacer\d+/ for toolbar tb-test; got: p12
12787 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_toolbar.xul | checkSet(76) correct ID p12 for toolbar tb-test****/^separator\d+/,/^spacer\d+/,/^separator\d+/,p11,/^spring\d+/,/^spacer\d+/,p12,/^spring\d+/,6, - "spring12790611592626" should equal "p12"
12788 INFO TEST-PASS | /tests/toolkit/content/tests/widgets/test_toolbar.xul | checkSet(76) extra fixed items for tb-test - 0 should equal 0
12789 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_toolbar.xul | checkSet(76) correct number of children for tb-test - 7 should equal 8
This failure seems to be happening on Mac every or almost every time since October 13. Before then, it was Windows only.
Most likely trigger for the explosion of instances is bug 553098 making it all faster (though I don't think I've ever seen anything where Windows was the only thing _fast_ enough to hit something before).
OS: Windows Server 2003 → All
Hardware: x86 → All
I think I have a fix for this.

What happens is that in toolbar.xml we do:

  case "separator":
  case "spring":
  case "spacer":
    newItem = document.createElementNS(XUL_NS, "toolbar" + aId);
    newItem.id = aId + Date.now() + this.childNodes.length;

Date.now() does not have a really high resolution, on Windows for example we know timers have about 16ms resolution, and on VMs this is often worse. So a lot of sub-tests will run with the same timestamp that I will call TIMESTAMP.

childNodes.length is there to solve this issue, but it changes along the test. not only increasing but also decreasing.

So in test 73 we have 2 nodes: p11, p12
in test 74 and 75 we have 5 nodes: separator,separator,p11,spring,spacer

what happens is that when the last spacer is created childNodes is 5 (because p12 will be removed later, see http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/toolbar.xml#244) so we create spacerTIMESTAMP5

in test 76 we have 8 nodes: separator,spacer,separator,p11,spring,spacer,p12,spring

notice that spacer is at position 6. Since timers suck when toolbar creates spacer it will create spacerTIMESTAMP5... WTF, this is the same we already inserted in position 2!

Solution is pretty simple, don't rely on childNodes.length, use a field instead!
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Whiteboard: [orange] → [orange][time-bomb]
Attached patch patch v1.0 — — Splinter Review 

        Attachment #486338 -
        Flags: review?(enndeakin)

    
  

        Attachment #486338 -
        Flags: review?(enndeakin) → review+

    
    

    
  
Comment on attachment 486338 [details] [diff] [review]
patch v1.0

a=beltzner

I'm fine with this landing through the beta7 freeze if it gets a clean run through tryserver. Feels like a good win to crack down on some orange.

        Attachment #486338 -
        Flags: approval2.0+

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
FWIW run on tryserver was fine, apart usual and known oranges:
http://tbpl.mozilla.org/?tree=MozillaTry&rev=47430d50b07b

I was asked if this could still be a problem for people who sets the clock in the past/future for testing. I think it should not be, the possibility of collision is bound to the event that the user creates the same exact type of toolbar element in the same exact millisecond with the same exact creation count.
This possibility was already small enough before for common users, just an issue in tests. Persisting a monotonic counter could be a greater cost compared to the gain.

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/eefdc0e5e208
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [orange][time-bomb] → [orange][time-bomb][relevant comments: 262,263,264,278,286]
Target Milestone: --- → mozilla2.0b7

    
    

    
      
  

    
    

    
  
comment 287 is before the fix push, fwiw

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  
Whiteboard: [orange][time-bomb][relevant comments: 262,263,264,278,286] → [time-bomb][relevant comments: 262,263,264,278,286]

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: