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

Is passing objects as reference from a coroutine to a coroutine function ok? #131

Closed
BurievSardor opened this issue Oct 30, 2022 · 3 comments

Comments

@BurievSardor
Copy link

BurievSardor commented Oct 30, 2022

An example:

QCoro::Task<> MyClass::doSomethingWithData(QByteArray &data) {

...........................................

QNetworkAccessManager networkAccessManager;
const QNetworkReply *reply = co_await networkAccessManager.get(url);
...............................................

}

QCoro::Task<> MyClass::fetchData() {

QNetworkAccessManager networkAccessManager;
const QNetworkReply *reply = co_await networkAccessManager.get(url);
const auto data = reply->readAll();
doSomethingWithData(data)

}

@danvratil
Copy link
Owner

Generally speaking, yes. It is OK to use references as coroutine arguments, but one has to be careful about the lifetime of the referenced object.

The example above may or may not be safe, since you did not show how you are using the data reference in doSomethingWithData().

Let me expand on your example:

QCoro::Task<> MyClass::doSomethingWithData(QByteArray &data) {
    QNetworkAccessManager networkAccessManager;
    // It's perfectly safe to use `data` up to this point
    // Actually, it's safe to use is on the next line as well, but only in the
    // expression *after* the `co_await` keyword:
    //                                    v---------------------------v
    const QNetworkReply *reply = co_await networkAccessManager.get(url);    
    // It's *NOT* safe to use `data` from this point on. The reference is dangling.
}

The reason is that the doSomethingWithData() coroutine is not co_awaited inside the fetchData() coroutine, so the following things happen:

  1. the data variable is allocated and initialized with result of reply->readAll()
  2. the doSomethingWithData() coroutine is called, passing it reference to data
  3. at this point, accessing the data reference inside doSomethingWithData() coroutine is completely safe
  4. the doSomethingWithData() coroutine is suspended when co_awaiting the network reply
  5. execution returns back to fetchData()
  6. fetchData() coroutine reaches the end of the function body, data is destroyed, networkAccessManager is destroyed, reply leaks ;-)
  7. at some point later, doSomethingWithData() coroutine is resumed
  8. at this point, data inside doSomethingWithData() coroutine is a dangling reference and must NOT be used, because the object that it references has been already destroyed in step 6.

IF the doSomethingWithData() coroutine were co_awaited inside fetchData(), then it would be totally safe to use the data reference anywhere inside the doSomethingWithData() coroutine, since the fetchData() coroutine would not be finished until the doSomethingWithData() coroutine would finish, and thus it would be guaranteed that the data object outlives the data reference.

To sum it up, yes, it's safe to pass arguments to coroutines as references, but one must be careful when using references whose lifespan crosses a suspension point (a co_await), as the may become dangling references while the coroutine is suspended.

@BurievSardor
Copy link
Author

BurievSardor commented Oct 31, 2022

Thank you for your immediate response and explanation. I now understand how to safely pass objects by reference to a coroutine function inside another coroutine function. It's my mistake that I forgot to add co_await keyword before doSomethingWithData() function.

@danvratil
Copy link
Owner

I should document this better in the docs :-)

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