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

Support for promise cancellation #26

Open
navrocky opened this issue Jun 16, 2019 · 7 comments
Open

Support for promise cancellation #26

navrocky opened this issue Jun 16, 2019 · 7 comments

Comments

@navrocky
Copy link

When I close window I want to cancel all pending promises, but cannot figure how can do this?

@pwuertz
Copy link
Contributor

pwuertz commented Jun 17, 2019

What exactly do you mean by "cancel"?

If you just don't want the promise continuations (then/fail) to be invoked, stopping the event loop and destroying all QObjects is probably enough.

Or are you looking for a cancel function that is supposed to stop whatever the asynchronous task is? QtPromise currently doesn't have an interface for encapsulating cancellations. You'd have to create a cancellation function (e.g. a lambda for cancelling a specific download or stopping a thread), store it along with the promise, call the cancellation function later, finally await the promise for the cancelled task since cancellation might be async too.

@navrocky
Copy link
Author

If you just don't want the promise continuations (then/fail) to be invoked, stopping the event loop and destroying all QObjects is probably enough.

Yes, I mean such cancellation. But stopping event loop is a bad idea, I think, if user close some dialog with a long time query running.

@pwuertz
Copy link
Contributor

pwuertz commented Jun 17, 2019

Oh ok, by "close window" I thought you meant shutting down the application. So you have some kind of dialog with a long running promise and you want that promise to be cancelled once the window is closed? I think the best option is to connect a lambda to the appropriate dialog signal and call the promise reject() functor from there. The promise gets rejected with an exception of your choice. You can then check for that specific exception and handle it in any way you like (e.g. ignore it).

@LanderN
Copy link

LanderN commented Aug 20, 2019

What about a .cancel() method which will internally call the reject functor with QPromiseCanceledException() as an argument?

I do wonder if it would be useful to NOT invoke the default fail handler on cancel.
Right now, we would need to do this to prevent the default fail handler from invoking:

promise
.then([]() {
    // resolve handler
})
.fail([](QtPromise::QPromiseCanceledException) {}) // no fail on cancel
.fail([]() {
    // actual fail handler
});

connect(dialog, &QDialog::rejected,
        &promise, &QPromise::cancel);

@Poldraunic
Copy link
Contributor

Poldraunic commented Jul 11, 2022

I support this.

Currently, it is a hassle to keep track of future execution. Here is the situation I often encounter in my app:

void Widget::getData() {
  m_dataFuture.cancel();

  QFuture<Data> dataFuture = m_dataProvider->fetchData();
  QtPromise::resolve(dataFuture)
    .then([this, dataFuture](const Data &data) { if (!future.isCanceled()) setData(data) })
    .fail([](){});

  m_dataFuture = dataFuture;
}

void Widget::~Widget() {
  m_dataFuture.cancel();
}

This situation can be easily encountered by user closing widget before execution finishes. If I don't capture actual future and check for its state, app will crash on setting data into widget thats been already destroyed.

What I would like to see is a mechanism to cancel/disarm/whatever-you-want-to-call-it all promise callbacks. Pretty often I have to keep track of QFuture object when in reality I don't really need it and I would like to return QtPromise::QPromise straight out of my function.

What would be even better, is to control life-time of the QtPromise::QPromise object using QObject system, but I think it would be too much and introduce dependency on QObject. But then again, I can make a helper on my side that does connect(object, &QObject::destroyed, &promise, &QtPromise::QPromise::cancel) pretty easily.

@penn-thomas
Copy link

I support this.

Currently, it is a hassle to keep track of future execution. Here is the situation I often encounter in my app:

void Widget::getData() {
  m_dataFuture.cancel();

  QFuture<Data> dataFuture = m_dataProvider->fetchData();
  QtPromise::resolve(dataFuture)
    .then([this, dataFuture](const Data &data) { if (!future.isCanceled()) setData(data) })
    .fail([](){});

  m_dataFuture = dataFuture;
}

void Widget::~Widget() {
  m_dataFuture.cancel();
}

This situation can be easily encountered by user closing widget before execution finishes. If I don't capture actual future and check for its state, app will crash on setting data into widget thats been already destroyed.

What I would like to see is a mechanism to cancel/disarm/whatever-you-want-to-call-it all promise callbacks. Pretty often I have to keep track of QFuture object when in reality I don't really need it and I would like to return QtPromise::QPromise straight out of my function.

What would be even better, is to control life-time of the QtPromise::QPromise object using QObject system, but I think it would be too much and introduce dependency on QObject. But then again, I can make a helper on my side that does connect(object, &QObject::destroyed, &promise, &QtPromise::QPromise::cancel) pretty easily.

I use QPointer like this:

QPointer<QObject> local(this);
promise.then([local]() {
  if (local.isNull()) {
    return;
  }
  ......
}).fail([local]() {
  if (local.isNull()) {
    return;
  }
  ......
});

This avoid most crash, but it cannot immediately reject the promise when 'close the dialog/widget', and need many if in every fail/then/finally, so it's still not a perfect way.

@Poldraunic
Copy link
Contributor

I use QPointer like this:

QPointer<QObject> local(this);
promise.then([local]() {
  if (local.isNull()) {
    return;
  }
  ......
}).fail([local]() {
  if (local.isNull()) {
    return;
  }
  ......
});

This avoid most crash, but it cannot immediately reject the promise when 'close the dialog/widget', and need many if in every fail/then/finally, so it's still not a perfect way.

Would you believe that! I came up with this solution today as well, just couple of hours before your post.

After going through library code today, I figured my suggestion wouldn't help too much either and was planning on writing another comment later today. Anyhow, what I figured out, is that when my promise gets fullfilled and handler events are scheduled, my object gets destroyed and this results in a crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants