Repository navigation
default thread timeout too short for CI #1339
Description
Since this morning our CI started failing with:
Run python devtools/autorelease_check.py
Traceback (most recent call last):
File "devtools/autorelease_check.py", line 24, in <module>
repo_path='.'
File "/usr/share/miniconda/envs/test/lib/python3.7/site-packages/autorelease/check_runners.py", line 52, in __init__
repo_path=repo_path
File "/usr/share/miniconda/envs/test/lib/python3.7/site-packages/autorelease/git_repo_checks.py", line 28, in __init__
self.repo.remotes.origin.fetch()
File "/usr/share/miniconda/envs/test/lib/python3.7/site-packages/git/remote.py", line 856, in fetch
res = self._get_fetch_info_from_stderr(proc, progress)
File "/usr/share/miniconda/envs/test/lib/python3.7/site-packages/git/remote.py", line 727, in _get_fetch_info_from_stderr
handle_process_output(proc, None, progress_handler, finalizer=None, decode_streams=False)
File "/usr/share/miniconda/envs/test/lib/python3.7/site-packages/git/cmd.py", line 151, in handle_process_output
raise RuntimeError(f"Thread join() timed out in cmd.handle_process_output(). Timeout={timeout} seconds")
RuntimeError: Thread join() timed out in cmd.handle_process_output(). Timeout=10.0 seconds
Fatal Python error: could not acquire lock for <_io.BufferedReader name=4> at interpreter shutdown, possibly due to daemon threads
Thread 0x00007fbb2f2d7700 (most recent call first):
File "/usr/share/miniconda/envs/test/lib/python3.7/site-packages/git/cmd.py", line 103 in pump_stream
File "/usr/share/miniconda/envs/test/lib/python3.7/threading.py", line 870 in run
File "/usr/share/miniconda/envs/test/lib/python3.7/threading.py", line 926 in _bootstrap_inner
File "/usr/share/miniconda/envs/test/lib/python3.7/threading.py", line 890 in _bootstrap
Current thread 0x00007fbb3332d180 (most recent call first):
File "/usr/share/miniconda/envs/test/lib/python3.7/site-packages/git/cmd.py", line 406 in __del__
/home/runner/work/_temp/ff824c06-ff56-4ee4-9e2a-e65258688cf4.sh: line 1: 1922 Aborted (core dumped) python devtools/autorelease_check.py
Error: Process completed with exit code 134.
This seems a side effect of the timeout introduced by #1318
Could this time-out default to 60 s instead (or be taken and propagated by the fetch function)?
I could make a PR if needed, with either of the two solutions
Activity
sroet commentedon Sep 10, 2021
cc: @dwhswenson
Byron commentedon Sep 10, 2021
Thanks for reporting and sorry for the hassle. Does that timeout not also means that clones or fetches can't take longer than that before assuming a hang? CC @Yobmod
Byron commentedon Sep 10, 2021
@sroet A PR would definitely be welcome, even though it would be good to understand it fully first.
LasseMenzel commentedon Sep 10, 2021
Hi, same on my CI-System, where fetches takes long time and 10 sec time-out are definitive to short. A default value of 60 sec for time-out are also fine for me.
dwhswenson commentedon Sep 10, 2021
The failure @sroet linked to in OpenPathSampling is raised from within Autorelease, which includes a tool to test that the version listed in the code is reasonable based on the versions implied by tags in the repo. Autorelease uses GitPython (thanks for a great package for interacting with git from Python!), and the problem here is when it fetches the upstream to ensure it knows all tags.
I use Autorelease on all my projects, and OpenPathSampling is the only one where this is a problem. I suspect this is because OpenPathSampling is a relatively large repository (both in clone size and in number of commits) -- though there are plenty of repos out there that are larger in each.
In previous passing nightly builds, total time for the "Autorelease check" step in the OpenPathSampling GitHub actions runs varies widely: I've seen as long as 1m27s, but that seems like an outlier. ~15s is standard. For my smaller repos, the "Autorelease check" step takes more like 2-5 seconds.
So a few thoughts:
timeoutthat will satisfy all users. If you're working with a sufficiently massive repo, this will be a problem. OpenPathSampling is large, but not an extreme outlier.timeoutparameter inRemote.fetch(and everything else that useshandle_process_output) or using an approach similar to updating thecmd.execute_kwargsdict would make it easier for me to implement a workaround. (For now, the easy solution is going to be pinning to an old version of GitPython.)handle_process_outputwould ever be less thanexecute_kwargs['kill_after_timeout']? Is there a reason not to reuse the value ofexecute_kwargs['kill_after_timeout']inhandle_process_output? This would default to no timeout, but would give client code a straightforward way to change this without adding confusion due to different kinds of timeouts.Byron commentedon Sep 10, 2021
Thanks for the elaborate explanation and for sharing your thoughts, I found them very insightful and helpful.
I am not so sure about that anymore. Even though I don't know every possible usage of it, I think that it could have been at its best if it would have stuck to executing git and parsing its output, but do that very, very well. GitPython today is a deeply flawed hybrid which wants to be git, and probably beyond fixable.
For a start, I have yanked this release as the timeout, no matter how long, represents a breaking bug.
It looks like there is a PR by now, maybe @dwhswenson could take a look at #1340 as well. Thank you.