Subject: Re: [bug#73664] [PATCH] gnu: libtorrent-rasterbar: Work around hang in test_ssl.

From: Maxim Cournoyer <[email-masked]>
Date: Wed, 29 Jan 2025 11:00:45 +0900

Hi Tomas,

Tomas Volf <[email-masked]> writes:

> Hi Maxim,
>
> thank you very much for the review and apologies for the late response,
> I have kept postponing it.
>
> Maxim Cournoyer <[email-masked]> writes:
>
>> Hi Tomas,
>>
>> Tomas Volf <[email-masked]> writes:
>>
>>> test_ssl does sometimes hang (at least when executed under faketime).  It is
>>> somewhat unlikely to happen, and (on my machine) required a build with
>>> --rounds=32 to reproduce it.
>>
>> It'd be nice if upstream was made aware of this problem.  Perhaps they
>> could come up with a fix for good.
>
> My experience with reporting bugs to this specific upstream is not the
> best, and I admit I expect the answer to "it hangs under faketime" would
> be "do not run it under faketime".

Some upstream takes patience to work with, but I still think it's useful
to put out a report of this problem, would it only be for other
downstreams trying to find a solution.

In this case, I'd tell them the goal is to have the tests work reliably
in time, even in 2 or 5 years, and this is not possible currently
because of expiry on the certs they use in the test suite (right?).

Possible options:

1. Remove expiry on certs used.
2. Use faketime, as attempted here; if this is chosen we need to figure
out why it hangs.

Obviously 1 is the easier/better option (if it's possible).

>>
>>> The workaround is to set somewhat lower timeout of 240s (expected test
>>> duration * 5 rounded up to whole minutes) and retry few times on failure.  In
>>> this way, --rounds=64 finished successfully (on my machine).
>>>
>>> At the same time remove the timeout from the other tests, since it is not
>>> necessary (they do not hang), and one of them runs for ~270s (almost half the
>>> original timeout), so it could pose a problem on slow/overloaded machine.
>>
>> This means the tests may take up to 20 minutes, which is a bit too much
>> to my taste.
>
> During validation of the v2, the run-time of the check phase is:
>
> phase `check' succeeded after 811.0 seconds
>
> That seems fine?  We have software already that takes similar time (or
> even longer) to build.

That's still too long to my taste, but if it succeeds reliably at least,
then so be it.

>> I think that a test sometimes hang is a good reason to leave it
>> disabled, report it to upstream, and reference the issue.  The test can
>> be re-enabled when the issue is resolved and part of a new release.
>>
>> So in concrete terms, what I'd rather see here is a report of the
>> problems (requirement on faketime + propension to hang) to upstream, the
>> and an updated comment cross-referencing it (with the test kept
>> commented-out/disabled in the mean time).
>>
>> Does that make sense?
>
> As mentioned above, I do not expect upstream to care about this specific
> issue (faketime).  However what I was successful in was convincing
> upstream to vastly increase the lifetime of the bundled SSL certificates
> used for testing.  That means that the new version (2.0.11) will run the
> tests fine until ~end of the year 2297.  Honestly I consider that to be
> an acceptable deadline to find a better long term solution (for example
> we could explore the use of time namespace for build environment).

That's good!  That's practically the #1 option I hinted at earlier
(didn't know they already did that).

> So I have submitted v2 which updates to the new version and removes all
> special casing for test_ssl, since it should not be required for more
> than quarter of a millennium.
>
> I hope this approach is acceptable.

Great!  Thanks for engaging with upstream and sending a v2.  I'll apply
it shortly if everything looks good.

-- 
Thanks,
Maxim