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

Do not lock around writing to stdout, do not flush #1411

Open
wants to merge 6 commits into
base: devel
Choose a base branch
from

Conversation

sivel
Copy link
Member

@sivel sivel commented Jan 22, 2025

Do not lock around writing to stdout, do not flush

@sivel sivel force-pushed the no-lock-no-flush branch 5 times, most recently from a2aaf2a to 2279033 Compare January 23, 2025 00:09
@sivel sivel force-pushed the no-lock-no-flush branch 2 times, most recently from a7f8aa2 to a6e8a45 Compare February 10, 2025 18:29
@github-actions github-actions bot added the test Changes to test files label Feb 10, 2025
@github-actions github-actions bot removed the test Changes to test files label Feb 10, 2025
@sivel sivel closed this Feb 10, 2025
@sivel sivel reopened this Feb 10, 2025
def _getcallargs(sig: inspect.Signature, *args: P.args, **kwargs: P.kwargs) -> types.MappingProxyType:
ba = sig.bind(*args, **kwargs)
ba.apply_defaults()
return types.MappingProxyType(ba.arguments)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff is unnecessary for this PR, and if desired I can remove the last commit. Looking at things like this just drove me a little crazy:

log_only = args[5] if len(args) >= 6 else kwargs.get('log_only', False)

@sivel sivel marked this pull request as ready for review February 10, 2025 19:15
@sivel sivel requested a review from a team as a code owner February 10, 2025 19:15
@sivel
Copy link
Member Author

sivel commented Feb 10, 2025

Reproducer for deadlock:

runme.py

import os
import ansible_runner

r = ansible_runner.run(
    private_data_dir=os.path.abspath(os.getcwd()),
    playbook='playbook.yml',
    inventory='hosts',
    forks=50,
    settings=dict(
        suppress_ansible_output=True,
    ),
)

project/action_plugins/chatty.py

from __future__ import annotations

from ansible.plugins.action import ActionBase
from ansible.utils.display import Display

display = Display()


class ActionModule(ActionBase):
    def run(self, tmp=None, task_vars=None):
        host = task_vars['inventory_hostname']
        for i in range(100):
            for a in ('display', 'v', 'vv', 'vvv', 'vvvv', 'vvvvv', 'vvvvvv', 'verbose', 'warning', 'debug'):
                getattr(display, a)(f'<{host}> chatty {i=} {a=}')
        return {}

Create an inventory with 100 hosts, and a playbook just like:

- hosts: all
  gather_facts: false
  tasks:
    - chatty:

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

Successfully merging this pull request may close these issues.

1 participant