quickfuture icon indicating copy to clipboard operation
quickfuture copied to clipboard

seqfault in #define QF_WRAPPER_CONNECT(method, checker)

Open swex opened this issue 7 years ago • 5 comments

Hi thank you for useful lib. Trying to use it in my application. And got crash in

    QF_WRAPPER_CONNECT(onFinished, isFinished)

I can see in debugger that watcher is null, when it funished signal emmited don't know how its possible. It probably related to next scenario: 0. StackView, with single Item

  1. Somewhere in that page I'm calling QFuture<QVariantMap> slot and wrapping it with Future.onFinished
  2. I'm going out of that page (replacing it with another) while QFuture thread still running
  3. Page probably deleted with qquick GC
  4. After QFuture finished some strange thing happend I've got crash in qfvariantwrapper.h

swex avatar May 18 '18 23:05 swex

Hi,

The null watcher is a bug (and I will fix it) but I think that is not the root cause of your crash issue. I guess QF_WRAPPER_CONNECT statement is not the top item of the backtrace record. It should be crashed within the callback you have passed to onFinished, which accesses the member from the destroyed page. Therefore it crashes.

I may modify the API to allow to add an optional owner argument to the onFinished API.

Future.onFinished(future, callback, owner)

// owner: It is an optional argument. If it is assigned, the callback will be disconnected automatically once the owner is destroyed. It prevents an unexcepted crash due to the destruction of the owner QML item.

Then you may put the page item to the owner field to prevent the callback execution after the page destruction. What do you think about the changes?

benlau avatar May 19 '18 04:05 benlau

I think that's great!

swex avatar May 19 '18 08:05 swex

Also how do you like next syntax

Future {
        id: futureItem
        onFinished: {
            console.log("finished", result)
        }
    }
    function example() {
        futureItem.future = CApi.callFutureFunction()
    }

I assume that as futureItem destroyed all connections would be destroyed, so no reason to pass owner explicitly

swex avatar May 19 '18 08:05 swex

checkout this its demonstration of prev. comment its context aware and very simple

swex avatar May 19 '18 21:05 swex

hmm... Let's me think about it, but I prefer to make a simple solution first. I have added the argument "owner" to onFInished / onCanceled call. You may try about it.

benlau avatar May 21 '18 08:05 benlau