pre-receive timeout is not precise enough

[agateau-gg]

Environment

• ggshield version: 1.13.6
• Operating system (Linux, macOS, Windows): Linux
• Operating system version: Ubuntu 20.04
• Python version: 3.8

Describe the bug

ggshield secret scan pre-receive is supposed to stop after a predefined timeout, which defaults to 4.5s by default. However it can sometimes take more than twice this time to stop.

Steps to reproduce:

1. Setup a local bare repository with a pre-receive hook calling ggshield
2. Clone it
3. Make your internet connection slow enough to hit the timeout (For example use wondershaper to limit the bandwidth of your network interface to 2KBps)
4. Measure the timeout

Actual result:

It took 10s for the hook to stop on my machine.

Expected result:

The difference between the defined timeout and the actual delay should be less than 1s.

Possible explanation

I suspect this is because the timeout is implemented using a thread, and Python GIL gets in the way. If this is the case, then that bug can be solved by running the scan in a different process.

[Walz]

The timeout is enforced by the class ExitAfter which start a thread after n seconds which calls thread.interrupt_main(). But this does not stop the subthreads started by scan_commit_range with a ThreadPoolExecutor.

The behavior can be reproduced with this small script:

import time
from concurrent.futures import ThreadPoolExecutor, as_completed

from ggshield.cmd.secret.scan.prereceive import ExitAfter


def sleep_fn(s, x):
    for i in range(x):
        print(f"thread#{s} sleep#{i}")
        time.sleep(1)


if __name__ == "__main__":
    start_at = time.time()
    try:
        with ExitAfter(2):
            with ThreadPoolExecutor(max_workers=2) as executor:
                fs = [executor.submit(sleep_fn, i, 4) for i in range(3)]
                print(list(as_completed(fs)))
    finally:
        print(f"took {time.time() - start_at}")

Output:

thread#0 sleep#0
thread#1 sleep#0
thread#0 sleep#1
thread#1 sleep#1

Pre-receive hook took too long
thread#1 sleep#2
thread#0 sleep#2
thread#1 sleep#3
thread#0 sleep#3
thread#2 sleep#0
thread#2 sleep#1
thread#2 sleep#2
thread#2 sleep#3
took 8.032462120056152
Traceback (most recent call last):
  File "/Users/sguillaume/Library/Application Support/JetBrains/PyCharm2022.3/scratches/scratch_15.py", line 19, in <module>
    print(list(as_completed(fs)))
  File "/Users/sguillaume/.pyenv/versions/3.10.10/lib/python3.10/concurrent/futures/_base.py", line 245, in as_completed
    waiter.event.wait(wait_timeout)
  File "/Users/sguillaume/.pyenv/versions/3.10.10/lib/python3.10/threading.py", line 607, in wait
    signaled = self._cond.wait(timeout)
  File "/Users/sguillaume/.pyenv/versions/3.10.10/lib/python3.10/threading.py", line 320, in wait
    waiter.acquire()
KeyboardInterrupt

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/sguillaume/Library/Application Support/JetBrains/PyCharm2022.3/scratches/scratch_15.py", line 16, in <module>
    with ExitAfter(2):
  File "/Users/sguillaume/gg-src/ggshield/ggshield/cmd/secret/scan/prereceive.py", line 64, in __exit__
    raise TimeoutError()
TimeoutError

The timeout after 2 seconds is not respected and the threads are allowed to run to completion.

Possible solution

We can share a threading.Event between the threads. Then each thread will be responsible for gracefully terminating when the event is set. This can be done in the for commit in commits loop of ggshield.scan.repo.scan_commits_content (the function executed by the threads).

[agateau-gg]

Sounds like a good idea. This would be less intrusive than my idea of using processes instead of threads.