strace test side-effect hangs selftest

Bug #226769 reported by Vincent Ladeuil
2
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
Medium
Vincent Ladeuil

Bug Description

While submitting a merge request to pqm, selftest hanged.

It turned out the submission revealed an hidden bug (probably related to bug #103133).

I was able to reproduce it on gutsy (with the patch proposed at http://bundlebuggy.aaronbentley.com/request/%<email address hidden>%3E applied) with:

python2.4 bzr selftest bzrlib.tests.test_strace.TestStrace.test_strace_callable_is_called bzrlib.tests.test_strace.TestStrace.test_strace_callable_result bzrlib.tests.blackbox.test_serve.TestBzrServe.test_bzr_connect_to_bzr_ssh

Ommiting any of the three tests doesn't exhibit the hang. IMHO, that's sufficient to demonstrate the correlation between a test (bzrlib.tests.blackbox.test_serve.TestBzrServe.test_bzr_connect_to_bzr_ssh) leaving a subprocess and probably one or two threads alive after completion and an invocation of strace on a process which is itslef the parent of the strace process, said process being killed.

Researching strace and ptrace man pages (and bug #103133) suggests that we may be trying to use strace in a configuration that was not taken into account in its design.

Several ways to address the initial problem come to mind:

1) Disable strace tests for python2.4 (I wasn't able to reproduce it with python 2.5 but it occured on the pqm machine with 2.4.2 and I can reproduce it with 2.4.4)

2) Fix TestBzrServe.test_bzr_connect_to_bzr_ssh so that the tearDown method does a better job at shutting down its server,

3) Implement a protection mechanism in selftest as proposed in https://bugs.launchpad.net/bzr/+bug/69978/comments/2

I'll bring the subject to the mailing list for discussion.

Vincent Ladeuil (vila)
Changed in bzr:
assignee: nobody → vila
importance: Undecided → Medium
status: New → Confirmed
Revision history for this message
Martin Pool (mbp) wrote : Re: [Bug 226769] [NEW] strace test side-effect hangs selfltest

On Mon, May 5, 2008 at 4:51 PM, vila <email address hidden> wrote:
> Public bug reported:
>
> While submitting a merge request to pqm, selftest hanged.

Isn't English bizarre? <http://englishplus.com/grammar/00000278.htm>

> Researching strace and ptrace man pages (and bug #103133) suggests that
> we may be trying to use strace in a configuration that was not taken
> into account in its design.

I think so too, and I said so last time this came up.

I don't think we even have any current code that makes use of the
strace function. Perhaps the most practical thing is just to disable
or delete it? If we do want to retain it for use in
testing/profiling, I think it would be reasonable to leave it in but
untested, and allow people to fix it if it is broken when they need
it.

--
Martin <http://launchpad.net/~mbp/>

Revision history for this message
Vincent Ladeuil (vila) wrote :

>>>>> "martin" == Martin Pool <email address hidden> writes:

    martin> On Mon, May 5, 2008 at 4:51 PM, vila <email address hidden> wrote:
    >> Public bug reported:
    >>
    >> While submitting a merge request to pqm, selftest hanged.

    martin> Isn't English bizarre? <http://englishplus.com/grammar/00000278.htm>

Lol, funnily enouh, I was wondering yesterday about the double
meaning suspended/killed :) Especially considering that in french
it's suspendre (to suspend) /pendre (to hang)...

So "We hung the towels out on the clothesline to dry." (from
above) is really what selftest was doing then, waiting for the
tests to dry :)

    >> Researching strace and ptrace man pages (and bug #103133)
    >> suggests that we may be trying to use strace in a
    >> configuration that was not taken into account in its
    >> design.

    martin> I think so too, and I said so last time this came up.

    martin> I don't think we even have any current code that
    martin> makes use of the strace function.

Indeed, grep returns no reference outside of strace.py and test_strace.py.

    martin> Perhaps the most practical thing is just to disable
    martin> or delete it?

Deleting it may be a bit extreme, we can still hope that strace
will be fixed...

    martin> If we do want to retain it for use in
    martin> testing/profiling, I think it would be reasonable to
    martin> leave it in but untested, and allow people to fix it
    martin> if it is broken when they need it.

Ok, I'll submit a patch skipping the tests with a comment
pointing here then.

Vincent Ladeuil (vila)
Changed in bzr:
status: Confirmed → Fix Committed
Vincent Ladeuil (vila)
Changed in bzr:
milestone: none → 1.5
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.