330763 - Tinderbox CVS checkouts should always use timeouts

Status: RESOLVED FIXED

(Webtools Graveyard :: Tinderbox, defect)

Description

[Mark Mentovai]

Followup from bug 329573 comment 8: tinderbox doesn't use a timeout when checking out client.mk.

Comment 1

[Mark Mentovai]

Attachment: Use a timeout when checking out client.mk

(untested so far)

This overloads $CVSCheckoutTimeout, which is currently used as the timeout for running "make -f client.mk checkout" (to check out the tree to build).  That doesn't seem like a problem: the alternative, multiple timeouts for checking things out in tinder-config, seems confusing.  On the other hand, it's conceivable that you might want a much shorter timeout for checking out one file than you would to check a good chunk of the repository.  I haven't really made up my mind on this yet.

Also, I didn't implement the

                    # Need to figure out how to kill rogue cvs processes
                    my $_cvs_pid=`ps -u $ENV{USER} | grep cvs`;
...

that's used for the tree checkout.  The reason for this is that wait_for_pid already kills a timed-out process by using its known pid from fork time.  If that doesn't work (and I suspect that it doesn't because I sometimes find errant cvs processes hanging around doing nothing), neither will a lame little ps/kill combination.  Maybe wait_for_pid should be sending a stronger signal.

Comment 2

[Robert Kaiser]

Something has to be wrong with timeouts in current tinderbox trunk for sure, as my Windows tinderbox (tpol on the SeaMonkey branch trees) every once in a while has hanging cvs process that doesn't get killed, but somehow the build process continues. That makes tinderbox fail in the following cycles as cvsco.log stays locked from that still hanging cvs process. Killing the cvs process from Windows task mananager causes everything to go normal again.
Cygwin cvs, rarely even rsync, seems to get stuck sometimes, possibly due to frequent network collisions (tpol is on an old-style hub together with two Linux tinderboxen, their tools seem to cope better with the situation).

So for me, it looks like we're sometimes having trouble killing the processes, even if we're in some way timing out and continuing the tinderbox script.

Is this connected with the problem here, or should I perhaps file another bug for that?

Comment 3

[Mark Mentovai]

Bug 330779 to cover tinderbox' inability to perform infanticide.

Comment 4

[Mark Mentovai]

Comment on attachment 215355 [details] [diff] [review]
Use a timeout when checking out client.mk

This has now been tested.

Comment 5

[J. Paul Reed [:preed]]

Comment on attachment 215355 [details] [diff] [review]
Use a timeout when checking out client.mk

Ahh... unless()... one of my most hated perl operator.

r=preed.

Comment 6

[Mark Mentovai]

Me too!

I've flipped it to if-not for checkin.