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