Resolving the last of the selftest thread hangs

Vincent Ladeuil v.ladeuil+lp at free.fr
Thu Oct 7 12:27:28 BST 2010


>>>>> Martin (gzlist) <gzlist at googlemail.com> writes:

    > On 07/10/2010, Martin Pool <mbp at canonical.com> wrote:
    >> 
    >> I don't understand how this makes things slower?  Is it because the
    >> transports never get gc'd, therefore hang around clogging up memory?
    >> I'm a little surprised the effect is so extreme.

    > Vincent can correct me on the details here, but basically the fix for
    > leaking threads is to make sure you join them.

After properly shutting down the sockets which is where the hell begins,
but roughly yes.

    > However some tests using socket were getting wedged which was
    > adding the timeout duration per thread used.

Indeed. Note that the delay occurred only on windows (AFAICS), on OSX
the the symptom was different, sockets were still leaked.

    > The good news is fixing get_transport really does resolve this
    > problem,

More precisely, by adding a level of indirection, we avoid the aliasing
problem created by importing the 'get_transport' symbol in several
modules (which I overlooked when I fixed all such cases in the *test*
modules).

    > I asked vila to do a run with a hack to fix the hack, and we went
    > from:

    > <http://babune.ladeuil.net:24842/job/selftest-windows/190/>
    > "Took 1 hr 32 min"

    > To:

    > <http://babune.ladeuil.net:24842/job/selftest-subset-windows/8/>
    > "Took 45 min"

    > So it actually halves the runtime of the full suite on windows on
    > babune. Apparently also lets OSX actually complete for the first time.

Confirmed.

    >> More broadly than just tests, this seems to indicate that when we get
    >> a transport, we should perhaps do so within the context of a larger
    >> scope or context, and then it can be shut down by that.  That might be
    >> as simple as an ObjectWithCleanups; it goes towards the Context object
    >> pattern Robert wanted to introduce.  There's more than just tests that
    >> want to control the way transports are reused or disposed...

    > This sounds about right to me.

+1

In the mean time, replacing the hackish overrideAttr by a post_connect
hook in transports was the plan, I'm glad to see Martin's hack
demosntrates that the whole approach was correct (thanks again ;).

             Vincent



More information about the bazaar mailing list