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

Question: cocotb.task is not cancelable #3745

Closed
Febbe opened this issue Feb 28, 2024 · 5 comments
Closed

Question: cocotb.task is not cancelable #3745

Febbe opened this issue Feb 28, 2024 · 5 comments
Labels
type:question a support question

Comments

@Febbe
Copy link

Febbe commented Feb 28, 2024

I have a weird issue with some tasks wrapping coroutines passed to cocotb.triggers.First that they are neither killed or canceled.
Since one coroutine throws a SimTimeoutError and the other is the failing testcase, it is mandatory to kill the other coroutine when the first is done.
This is however implemented in First, but nothing happens, those coroutines just continue to run. Shouldn't an asyncio.CancelledError be raised in all the child coroutines?

I am using:
cocotb 1.8.1
cocotb-bus 0.2.1
Fedora release 38 (Thirty Eight)
python, sys, pyenv 3.11.7

async def with_timout(awaitable, awaitable_timout):
    print("started await with timeout")
    async def throw_timeout(awaitable_timout):
        print("await end in throw_timeout")
        await awaitable_timout
        print("timeout! raise exception")        
        raise cocotb.result.SimTimeoutError()         
    
    awt = cocotb.start_soon(awaitable)
    awtt = cocotb.start_soon(throw_timeout(awaitable_timout))
    return await cocotb.triggers.First(awt, awtt)

#[...]
await with_timout(test_end_cond(self), ClockCycles(cocotb.top.clk, 100))

When the timeout is not reached, the TB freezes, because the Clock is cleaned and the ClockCycles coroutine does not cancel its execution.

@ktbarrett
Copy link
Member

There are two issues here.

  1. Throwing CancelledError in cancelled tasks.

I just realized this isn't in an issue yet, but #2679 mentions it. It's on the schedule for 2.0.

  1. First should kill tasks.

This is contentious. Perhaps the user wants to potentially wait for a task to end, but doesn't want to kill it if it doesn't end. asyncio.wait does not kill child tasks, but lets them continue to run.

This is however implemented in First, but nothing happens

What you think is being cancelled is not what is being cancelled. The tasks in First that are cancelled are implementation details of First, they are not intended to cancel the tasks passed in.

@ktbarrett
Copy link
Member

The issue with the current design of tasks, that is also present in asyncio, is that tasks are allowed to be free rather than constrained to their use-case. It's very flexible, but really just complicates everything. Our plan for 2.0 is to design away from "free" tasks by categorizing free tasks started with cocotb.start_soon as symmetric to the test task, while the kinds of tasks used in First and Combine are asymmetric, which we will implement using a mechanism to enforce structured use, like is seen with trio's nursery. See #3583.

@Febbe
Copy link
Author

Febbe commented Feb 29, 2024

The more I think about this, the more it does make sense. Sorry for my confusions, I am learning python and cocotb right now, and I am used to C++ where all resources are cleaned due to RAII after they lose all of their lifetime.

Are all tasks stored in the scheduler with a weak reference, like in asyncio? I assume not, since the scheduler does not quit when I leave the main test coroutine and all handles are cleared with a running coroutine?

@ktbarrett
Copy link
Member

Tasks are global objects, they don't have local lifetime. asyncio Tasks are the same as cocotb's in this regard. The scheduler ends when the main test coroutine finishes. If it doesn't it's because there is a bug.

@Febbe Febbe changed the title cocotb.task is not cancelable because it's done already Question: cocotb.task is not cancelable Mar 1, 2024
@Febbe
Copy link
Author

Febbe commented Mar 1, 2024

I found the bug, I added a Timer in the hope to prevent the SIGSEG in vsimk, but that didn't helped and I accidentally added it as a timer of 1 sec, which is pretty long, since the second is simulated. After removing the FallingEdge trigger / just using the ClockCircle trigger, my TB works fine.
Thank you for your help.

@ktbarrett ktbarrett added the type:question a support question label Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:question a support question
Projects
None yet
Development

No branches or pull requests

2 participants