Opened 7 years ago

Closed 7 years ago

#2608 closed defect (fixed)

msgq process stays around

Reported by: shane Owned by: shane
Priority: medium Milestone: Sprint-20130122
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Medium
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 2 Add Hours to Ticket: 0.25
Total Hours: Internal?: no

Description

I discovered that on a slow system, boss will sometimes be unable to connect to msgq in time, and will shut down. This happens during the Lettuce tests on a Raspberry Pi, for example.

There is a bug, when the happens, which means that the msgq process is not shut down. The following change will perform this cleanup properly.

diff --git a/src/bin/bind10/bind10_src.py.in b/src/bin/bind10/bind10_src.py.in
index 882653b..3054d74 100755
--- a/src/bin/bind10/bind10_src.py.in
+++ b/src/bin/bind10/bind10_src.py.in
@@ -443,6 +443,8 @@ class BoB:
 
             # if we have been trying for "a while" give up
             if (time.time() - cc_connect_start) > self.msgq_timeout:
+                if msgq_proc.process:
+                    msgq_proc.process.kill()
                 logger.error(BIND10_CONNECTING_TO_CC_FAIL)
                 raise CChannelConnectError("Unable to connect to c-channel afte
 

Subtickets

Change History (4)

comment:1 Changed 7 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20130122

comment:2 Changed 7 years ago by jelte

  • Owner set to shane
  • Status changed from new to reviewing

Since there is already a patch, I'm jumping straight to review :)

Ideally, we'd make ProcessInfo? a Context class (where exit=kill), so that it's killed on any exception, not just this particular case). but I guess that'll be too much changes for this particular fix, and as such, I have confirmed it works as expected.

Tried shortly to see if we can make a test for it (since we do test it raises here) but the object is already gone by then, so I'm not sure how to.

Please merge this, does it need a changelog? (e.g. [bug] boss now kills the msgq process it spawns if it cannot immediately connect to it, or something).

BTW, one note about the diff; be careful when pasting git diff output from a terminal; as you can see it has the tendency to crop long lines. In this particular case the diff was simple and there were no problems applying it, and in most cases our lines aren't long anyway, but I have seen problems with the cropped diffs

comment:3 Changed 7 years ago by shane

Okay, merged and ChangeLog added.

comment:4 Changed 7 years ago by shane

  • Add Hours to Ticket changed from 0 to 0.25
  • Resolution set to fixed
  • Status changed from reviewing to closed
  • Total Hours 0 deleted
Note: See TracTickets for help on using tickets.