Ticket #2139 (closed defect: fixed)

Opened 19 months ago

Last modified 14 months ago

locks held by slaves are not released when slave goes missing

Reported by: dwlocks Owned by:
Priority: major Milestone: 0.8.6
Version: 0.8.4p2 Keywords:
Cc:

Description

It's a subtle bug. It only shows up when a slave goes missing while holding a lock. That lock is not released, and the slave (when it returns) does not pick up the same lock, so the lock is held until the buildmaster is restarted.

Change History

comment:1 Changed 18 months ago by dustin

  • Type changed from undecided to defect
  • Milestone changed from undecided to 0.8.6

comment:2 Changed 16 months ago by dustin

tomprince suggets

  • master/buildbot/buildslave.py 

    diff --git a/master/buildbot/buildslave.py b/master/buildbot/buildslave.py
index 9f06e65..b8c3c3a 100644
    a b class AbstractBuildSlave(config.ReconfigurableServiceMixin, pb.Avatar, 
    460460        log.msg("BuildSlave.detached(%s)" % self.slavename) 
    461461        self.botmaster.master.status.slaveDisconnected(self.slavename) 
    462462        self.stopKeepaliveTimer() 
     463 self.release() 
    463464 
    464465        # notify watchers, but do so in the next reactor iteration so that 
    465466        # any further detached() action by subclasses happens first 

which seems adequate to me, but is untested.

comment:3 Changed 16 months ago by Tom Prince

  • Status changed from new to closed
  • Resolution set to fixed

Release lock held by slave, when we lose the connection to the slave.

Fixes #2139

Changeset: f40f62cfcb9d456957dc179faced285d1699b1e2

comment:4 Changed 14 months ago by dwlocks

That should be self.releaseLocks()

It's committed in dwlocks/buildbot-0.8.4

7d0212ff8ec3f6d17c95e4df8fff493e29634682

  • master/buildbot/buildslave.py 

    diff --git a/master/buildbot/buildslave.py b/master/buildbot/buildslave.py
index e71d338..495544f 100644
    a b class AbstractBuildSlave(pb.Avatar, service.MultiService): 
    407407        log.msg("BuildSlave.detached(%s)" % self.slavename) 
    408408        self.botmaster.parent.status.slaveDisconnected(self.slavename) 
    409409        self.stopKeepaliveTimer() 
    410         self.release() 
     410        self.releaseLocks() 
    411411 
    412412    def disconnect(self): 
    413413        """Forcibly disconnect the slave. 
Last edited 14 months ago by dustin (previous) (diff)

comment:5 Changed 14 months ago by dustin

Tom fixed this up already, but thanks for clarifying:

  • master/buildbot/buildslave.py 

    commit 7d78de7d0b311ac34261a7e08eb951394c9ac734
Author: Tom Prince <tom.prince@ualberta.net>
Date:   Thu Jan 19 16:24:06 2012 -0500

    Fix stupid type s/release/releaseLocks/.

diff --git a/master/buildbot/buildslave.py b/master/buildbot/buildslave.py
index b8c3c3a..cd5dae7 100644
    a b class AbstractBuildSlave(config.ReconfigurableServiceMixin, pb.Avatar, 
    460460        log.msg("BuildSlave.detached(%s)" % self.slavename) 
    461461        self.botmaster.master.status.slaveDisconnected(self.slavename) 
    462462        self.stopKeepaliveTimer() 
    463         self.release() 
     463        self.releaseLocks() 
    464464 
    465465        # notify watchers, but do so in the next reactor iteration so that 
    466466        # any further detached() action by subclasses happens first 
Note: See TracTickets for help on using tickets.