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

Role dep cycle detection broken on Py3.7+ #61527

Open
nitzmahone opened this issue Aug 29, 2019 · 3 comments
Open

Role dep cycle detection broken on Py3.7+ #61527

nitzmahone opened this issue Aug 29, 2019 · 3 comments
Labels
affects_2.13 bug This issue/PR relates to a bug. P3 Priority 3 - Approved, No Time Limitation python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@nitzmahone
Copy link
Member

SUMMARY

Ansible's role cycle "detection" relied on the fact that stack overflows were recoverable in Python < 3.7- they'd catch (any old) RuntimeError and display a fatal message that a cycle was detected. This no longer works in 3.7+, and instead results in a crash with SIGABRT.

The recursive role dependency loader needs to explicitly detect cycles. Until then, the unit test for the cycle case in units.playbook.role.test_role.TestRole.test_load_role_with_metadata has been commented out.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

role metadata

ANSIBLE VERSION

2.9.0dev0

CONFIGURATION

NA

OS / ENVIRONMENT

NA

STEPS TO REPRODUCE

uncomment the cycle unit test above, and run it on Py3.7+

EXPECTED RESULTS

success

ACTUAL RESULTS

fail with SIGABRT

nitzmahone added a commit to nitzmahone/ansible that referenced this issue Aug 29, 2019
* failing on 3.7+, needs proper cycle detection
* see ansible#61527
@ansibot
Copy link
Contributor

ansibot commented Aug 29, 2019

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Aug 29, 2019
nitzmahone added a commit that referenced this issue Aug 29, 2019
* default collection support

* playbooks run from inside a registered collection will set that collection as the first item in the search order (as will all non-collection roles)
* this allows easy migration of runme.sh style playbook/role integration tests to collections without the playbooks/roles needing to know the name of their enclosing collection

* disable default collection test under Windows

* enable collection search for role dependencies

* unqualified role deps in collection-hosted roles will first search the containing collection
* if the calling role has specified a collections search list in metadata, it will be appended to the search order for unqualified role deps

* disable cycle detection unit test

* failing on 3.7+, needs proper cycle detection
* see #61527
adharshsrivatsr pushed a commit to adharshsrivatsr/ansible that referenced this issue Sep 3, 2019
* default collection support

* playbooks run from inside a registered collection will set that collection as the first item in the search order (as will all non-collection roles)
* this allows easy migration of runme.sh style playbook/role integration tests to collections without the playbooks/roles needing to know the name of their enclosing collection

* disable default collection test under Windows

* enable collection search for role dependencies

* unqualified role deps in collection-hosted roles will first search the containing collection
* if the calling role has specified a collections search list in metadata, it will be appended to the search order for unqualified role deps

* disable cycle detection unit test

* failing on 3.7+, needs proper cycle detection
* see ansible#61527
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Sep 5, 2019
anas-shami pushed a commit to anas-shami/ansible that referenced this issue Sep 23, 2019
* default collection support

* playbooks run from inside a registered collection will set that collection as the first item in the search order (as will all non-collection roles)
* this allows easy migration of runme.sh style playbook/role integration tests to collections without the playbooks/roles needing to know the name of their enclosing collection

* disable default collection test under Windows

* enable collection search for role dependencies

* unqualified role deps in collection-hosted roles will first search the containing collection
* if the calling role has specified a collections search list in metadata, it will be appended to the search order for unqualified role deps

* disable cycle detection unit test

* failing on 3.7+, needs proper cycle detection
* see ansible#61527
@s-hertel s-hertel added the affects_2.10 This issue/PR affects Ansible v2.10 label Dec 11, 2020
@WGH-
Copy link

WGH- commented Oct 24, 2021

I'm having this on CPython 3.8.12. I guess it's not limited to PyPy.

Abort happens here: https://github.com/python/cpython/blob/v3.8.12/Python/ceval.c#L691-L697

It seems more functions are being called when recursion error is being handled, because simple infinite recursions throw RecursionError just fine (on PyPy as well).

@nitzmahone nitzmahone added affects_2.13 and removed affects_2.9 This issue/PR affects Ansible v2.9 affects_2.10 This issue/PR affects Ansible v2.10 labels Jun 22, 2022
@s-hertel s-hertel added the P3 Priority 3 - Approved, No Time Limitation label Oct 5, 2022
@sivel
Copy link
Member

sivel commented Nov 30, 2022

As an update here, this is no longer an issue in Python 3.10. So at this point, it's only affecting Python 3.9. Although that's not to say we couldn't still catch this sooner, and not rely on RecursionError.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.13 bug This issue/PR relates to a bug. P3 Priority 3 - Approved, No Time Limitation python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

No branches or pull requests

6 participants