Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hangs (sometimes) if a checker raises an assertion failure #420

Open
wjt opened this issue May 3, 2024 · 1 comment
Open

Hangs (sometimes) if a checker raises an assertion failure #420

wjt opened this issue May 3, 2024 · 1 comment

Comments

@wjt
Copy link
Contributor

wjt commented May 3, 2024

In #418, @tobiasmicheler figured out that the checker was hanging while checking https://github.com/flathub/com.modrinth.ModrinthApp/blob/1d954cc7c91841d020fe4c860593789a16ca62c6/com.modrinth.ModrinthApp.yml#L108. This is because prior to 1a0e0c7, gitchecker.GitChecker._check_has_new asserted that the (user-supplied) regular expression contained exactly one match group.

Stylistically I think it was incorrect to make an assertion about user-supplied data: assertions are to catch logic errors in the checker itself. But nonetheless it is the case that assertions are used liberally in the various checkers, so it's a problem that the asyncio mainloop would hang in this situation.

The symptom is that once the assertion is raised, the program hangs; if you interrupt it with Ctrl+C you get the following backtrace:

^CTraceback (most recent call last):
  File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/base_events.py", line 653, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/sysroot/home/wjt/src/flathub-infra/flatpak-external-data-checker/src/main.py", line 465, in run_with_args
    await manifest_checker.check(args.filter_type)
  File "/sysroot/home/wjt/src/flathub-infra/flatpak-external-data-checker/src/manifest.py", line 408, in check
    ext_data_checked = await asyncio.gather(*check_tasks)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/sysroot/home/wjt/src/flathub-infra/flatpak-external-data-checker/src/manifest.py", line 343, in _check_data
    await checker.check(data)
  File "/sysroot/home/wjt/src/flathub-infra/flatpak-external-data-checker/src/checkers/gitchecker.py", line 92, in check
    return await self._check_has_new(external_data)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/sysroot/home/wjt/src/flathub-infra/flatpak-external-data-checker/src/checkers/gitchecker.py", line 101, in _check_has_new
    assert tag_re.groups == 1
           ^^^^^^^^^^^^^^^^^^
AssertionError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/sysroot/home/wjt/src/flathub-infra/flatpak-external-data-checker/./flatpak-external-data-checker", line 30, in <module>
    main()
  File "/sysroot/home/wjt/src/flathub-infra/flatpak-external-data-checker/src/main.py", line 504, in main
    outdated_num, errors_num, updated = asyncio.run(run_with_args(args))
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 189, in run
    with Runner(debug=debug) as runner:
  File "/usr/lib/python3.11/asyncio/runners.py", line 63, in __exit__
    self.close()
  File "/usr/lib/python3.11/asyncio/runners.py", line 71, in close
    _cancel_all_tasks(loop)
  File "/usr/lib/python3.11/asyncio/runners.py", line 201, in _cancel_all_tasks
    loop.run_until_complete(tasks.gather(*to_cancel, return_exceptions=True))
  File "/usr/lib/python3.11/asyncio/base_events.py", line 640, in run_until_complete
    self.run_forever()
  File "/usr/lib/python3.11/asyncio/base_events.py", line 607, in run_forever
    self._run_once()
  File "/usr/lib/python3.11/asyncio/base_events.py", line 1884, in _run_once
    event_list = self._selector.select(timeout)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/selectors.py", line 468, in select
    fd_event_list = self._selector.poll(timeout, max_ev)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyboardInterrupt

A bit of printf debugging shows that the gitchecker for another module is executing the async function git_ls_remote at the point of the hang. If I remove that module, the assertion bubbles up to the top level correctly.

@gasinvein
Copy link
Collaborator

gasinvein commented May 8, 2024

Stylistically I think it was incorrect to make an assertion about user-supplied data: assertions are to catch logic errors in the checker itself. But nonetheless it is the case that assertions are used liberally in the various checkers,

Indeed, assertions in cases such as this one are used in place of proper input checks and should be replaced, probably with schema-based validation (I believe the number of capture groups in a regex can be checked with an extended python-jsonschema validator).

so it's a problem that the asyncio mainloop would hang in this situation.

And this is kinda weird. Are there some specific cases when AssertionError isn't propagated to the main loop? Or anywhere in a coroutine will cause it to hang?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants