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

Add an @trio.not_in_async_task decorator, to error instead of stalling #2757

Closed
Zac-HD opened this issue Aug 17, 2023 · 5 comments · Fixed by #3195
Closed

Add an @trio.not_in_async_task decorator, to error instead of stalling #2757

Zac-HD opened this issue Aug 17, 2023 · 5 comments · Fixed by #3195

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Aug 17, 2023

In a large codebase, you're going to have some functions which are slow, and so you don't want them to be called from an async task (without using trio.to_thread.run_sync(slow_fn), anyway). flake8-trio's TRIO200 error can help find direct calls, but indirect calls are really hard to find via static analysis.

As I mentioned in my PyCon talk, and work we've implemented a decorator to mark sync functions which can be called in a sync context, but must be delegated to a worker thread instead of being called in an async context - no matter how many sync functions are in the intermediate call chain. Simple implementation:

def not_in_async_task(fn):
    """Decorator for sync functions which should not be called in an async context.

    <detailed docstring here>
    """
    @functools.wraps(fn)
    def decorator(*args, **kwargs):
        try:
            # Identical logic to trio.lowlevel.current_task()
            GLOBAL_RUN_CONTEXT.task
        except AttributeError:
            pass
        else:
            raise RuntimeError(
                f"{fn.__name__} should not be called in an async context.  "
                f"Use an async alternative, or trio.to_thread.run_sync({fn.__name__})."
            )
        return fn(*args, *kwargs)

    return decorator

Should we include this in trio itself? I've certainly found the pattern useful, and upstreaming it would encourage wider use of the pattern. On the other hand it's not entirely clear where this would fit, and it's neither hard to implement nor something which benefits much from standardization.

@oremanj
Copy link
Member

oremanj commented Aug 17, 2023

Perhaps the more fundamental primitive here is trio.in_trio_context(), which returns a bool. You can write it yourself using only public APIs, but it's weird:

def in_trio_context() -> bool:
    try:
        trio.current_time()
    except RuntimeError:
        return False
    return True

The private implementation could check for the existence of GLOBAL_RUN_CONTEXT.runner.

(There is a subtle difference between checking for runner and checking for task: the former exists even in Trio context outside of a task, which you can observe in abort_fns and instrument calls. I think the runner version is more correct for this need.)

@A5rocks
Copy link
Contributor

A5rocks commented Aug 2, 2024

I think this would be helpful. Given that debugging tools seem to be in trio.lowlevel typically, should this go there too?

@CoolCat467
Copy link
Member

I would agree that lowlevel would be a good home for in_trio_context

@A5rocks
Copy link
Contributor

A5rocks commented Jan 30, 2025

What's the expected behavior in guest runs? GLOBAL_RUN_CONTEXT.runner seems to exist there but... I don't think the host loop should be considered a trio context?

(maybe we should just tell people "use sniffio.current_async_library" because that has better semantics? I don't know what it returns for to_thread.run_sync calls though.)

@oremanj
Copy link
Member

oremanj commented Jan 30, 2025

In a guest run, as well as some other places like abort_fn calls and some instrument calls, you are in a Trio run (runner exists) but not in a Trio task (task does not exist). These concepts are separate but people tend to confuse them (and it doesn't help that Trio's error message is "must be run from async context" for both).

I attempted to make sniffio explicit about this using python-trio/sniffio#38 and the linked python-trio/sniffio#39 but it was never reviewed.

I think Trio should probably provide both lowlevel.in_trio_run and lowlevel.in_trio_task.

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 a pull request may close this issue.

4 participants