Crash with latest kasync master
Closed, ResolvedPublic

Description

I could reproduce it with kasync master and it vanished with kasync v0.3.0

Trace:   kube.query.resource..resource.config : Found match  "{fe78f40c-1310-4437-a8b9-95e1bc361f6a}"
Trace:   kube.resourceaccess                  : Starting access
Trace:   kube.query.resource.modelresult      : Received addition:  "{fe78f40c-1310-4437-a8b9-95e1bc361f6a}"
Trace:   kube.query.resource.modelresult      : Initial result set complete. Fetched all:  true
[New LWP 21]
Trace:   kube.notifier.resource.config        : Found match  "kolabnowCaldav"
Trace:   kube.kolabnowCaldav.resourceaccess   : Trying to connect
Trace:   kube.resourceaccess                  : Connecting to server  "kolabnowCaldav"
Trace:   kube.resourceaccess                  : Failed to connect to server  QLocalSocket::ServerNotFoundError "kolabnowCaldav"
Log:     kube.kolabnowCaldav.resourceaccess   : Starting resource  "/app/bin/sink_synchronizer" "kolabnowCaldav sink.caldav" Home path:  "/home/mollekopf"
[Detaching after fork from child process 22]
Trace:   kube.kolabnowCaldav.resourceaccess   : Started resource  "kolabnowCaldav" 23
Trace:   kube.kolabnowCaldav.resourceaccess   : Try to connect  "kolabnowCaldav"
Trace:   kube.resourceaccess                  : Connecting to server  "kolabnowCaldav"
Trace:   kube.resourceaccess                  : Failed to connect to server  QLocalSocket::ServerNotFoundError "kolabnowCaldav"

Thread 1 "kube" received signal SIGSEGV, Segmentation fault.
0x000055555556230b in std::__atomic_base<int>::operator--() ()
Log:     kolabnowCaldav.main       : Starting:  "kolabnowCaldav" "sink.caldav"
(gdb) Trace:   kolabnowCaldav.listener   : Trying to open  "kolabnowCaldav"
Trace:   kolabnowCaldav.listener   : "Listening on kolabnowCaldav"
Trace:   kolabnowCaldav.entitystore : Starting transaction:  1
Trace:   kolabnowCaldav.entitystore : Committing transaction
Trace:   kolabnowCaldav.synchronizer : Starting synchronizer:  "sink.caldav" "kolabnowCaldav"
Trace:   kolabnowCaldav.storage_lmdb : Creating database dynamically:  "kolabnowCaldav.changereplaydefault" true
Trace:   kolabnowCaldav.listener     : "Resource factory: 93824993161536"
Trace:   kolabnowCaldav.listener     : "\tResource: 93824993130240"
Log:     kolabnowCaldav.synchronizer : Secret not available but required.

(gdb)
(gdb)
(gdb)
(gdb)
(gdb)
(gdb)
(gdb) bt
#0  0x000055555556230b in std::__atomic_base<int>::operator--() ()
#1  0x00007ffff6419c06 in bool QAtomicOps<int>::deref<int>(std::atomic<int>&) () from /app/lib/libsink.so.0.9
#2  0x00007ffff64040f2 in QBasicAtomicInteger<int>::deref() () from /app/lib/libsink.so.0.9
#3  0x00007ffff66a7bc0 in QSharedPointer<QLocalSocket>::deref(QtSharedPointer::ExternalRefCountData*) () from /app/lib/libsink.so.0.9
#4  0x00007ffff66a4382 in QSharedPointer<QLocalSocket>::deref() () from /app/lib/libsink.so.0.9
#5  0x00007ffff66a1438 in QSharedPointer<QLocalSocket>::~QSharedPointer() () from /app/lib/libsink.so.0.9
#6  0x00007ffff66b728c in KAsync::Private::Executor<KAsync::ControlFlowFlag, QSharedPointer<QLocalSocket> >::executeJobAndApply(KAsync::Error const&, QSharedPointer<QLocalSocket>&&, std::function<KAsync::Job<KAsync::ControlFlowFlag> (KAsync::Error const&, QSharedPointer<QLocalSocket>)> const&, KAsync::Future<KAsync::ControlFlowFlag>&, std::integral_constant<bool, false>) () from /app/lib/libsink.so.0.9
#7  0x00007ffff66b4aca in KAsync::Private::Executor<KAsync::ControlFlowFlag, QSharedPointer<QLocalSocket> >::run(QSharedPointer<KAsync::Private::Execution> const&) () from /app/lib/libsink.so.0.9
#8  0x00007ffff66b2f30 in KAsync::Private::Executor<KAsync::ControlFlowFlag, QSharedPointer<QLocalSocket> >::runExecution(KAsync::Future<QSharedPointer<QLocalSocket> >*, QSharedPointer<KAsync::Private::Execution> const&, bool) ()
   from /app/lib/libsink.so.0.9
#9  0x00007ffff66b1166 in KAsync::Private::Executor<KAsync::ControlFlowFlag, QSharedPointer<QLocalSocket> >::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libsink.so.0.9
#10 0x00007ffff66b2325 in KAsync::Private::Executor<void, KAsync::ControlFlowFlag>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libsink.so.0.9
#11 0x00007ffff78ca194 in KAsync::Job<void>::exec() () from /app/lib/libkubeframework.so
#12 0x00007ffff66b7e35 in KAsync::Private::Executor<KAsync::ControlFlowFlag>::executeJobAndApply(std::function<KAsync::Job<KAsync::ControlFlowFlag> ()> const&, KAsync::Future<KAsync::ControlFlowFlag>&, std::integral_constant<bool, false>) ()
   from /app/lib/libsink.so.0.9
#13 0x00007ffff66b5857 in KAsync::Private::Executor<KAsync::ControlFlowFlag>::run(QSharedPointer<KAsync::Private::Execution> const&) () from /app/lib/libsink.so.0.9
#14 0x00007ffff66b36f4 in KAsync::Private::Executor<KAsync::ControlFlowFlag>::runExecution(KAsync::Future<void>*, QSharedPointer<KAsync::Private::Execution> const&, bool) () from /app/lib/libsink.so.0.9
#15 0x00007ffff66b1e0e in KAsync::Private::Executor<KAsync::ControlFlowFlag>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libsink.so.0.9
#16 0x00007ffff66b2325 in KAsync::Private::Executor<void, KAsync::ControlFlowFlag>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libsink.so.0.9
#17 0x00007ffff78ca194 in KAsync::Job<void>::exec() () from /app/lib/libkubeframework.so
#18 0x00007ffff669efcd in KAsync::doWhile(KAsync::Job<KAsync::ControlFlowFlag> const&)::{lambda(KAsync::Future<void>&)#1}::operator()(KAsync::Future<void>&) const () from /app/lib/libsink.so.0.9
#19 0x00007ffff66a3c7d in std::_Function_handler<void (KAsync::Future<void>&), KAsync::doWhile(KAsync::Job<KAsync::ControlFlowFlag> const&)::{lambda(KAsync::Future<void>&)#1}>::_M_invoke(std::_Any_data const&, KAsync::Future<void>&) ()
   from /app/lib/libsink.so.0.9
#20 0x00007ffff78f28f1 in std::function<void (KAsync::Future<void>&)>::operator()(KAsync::Future<void>&) const () from /app/lib/libkubeframework.so
#21 0x00007ffff78f2138 in KAsync::Private::Executor<void>::run(QSharedPointer<KAsync::Private::Execution> const&) () from /app/lib/libkubeframework.so
#22 0x00007ffff78f1b04 in KAsync::Private::Executor<void>::runExecution(KAsync::Future<void>*, QSharedPointer<KAsync::Private::Execution> const&, bool) () from /app/lib/libkubeframework.so
#23 0x00007ffff78f113a in KAsync::Private::Executor<void>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libkubeframework.so
#24 0x00007ffff78f0f49 in KAsync::Private::Executor<void>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libkubeframework.so
#25 0x00007ffff78f0f49 in KAsync::Private::Executor<void>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libkubeframework.so
#26 0x00007ffff78ca194 in KAsync::Job<void>::exec() () from /app/lib/libkubeframework.so
#27 0x00007ffff66b69ce in KAsync::Private::Executor<void, QSharedPointer<QLocalSocket> >::executeJobAndApply(KAsync::Error const&, QSharedPointer<QLocalSocket>&&, std::function<KAsync::Job<void> (KAsync::Error const&, QSharedPointer<QLocalSocket>)> const&, KAsync::Future<void>&, std::integral_constant<bool, true>) () from /app/lib/libsink.so.0.9
#28 0x00007ffff66b4286 in KAsync::Private::Executor<void, QSharedPointer<QLocalSocket> >::run(QSharedPointer<KAsync::Private::Execution> const&) () from /app/lib/libsink.so.0.9
#29 0x00007ffff66b2900 in KAsync::Private::Executor<void, QSharedPointer<QLocalSocket> >::runExecution(KAsync::Future<QSharedPointer<QLocalSocket> >*, QSharedPointer<KAsync::Private::Execution> const&, bool) () from /app/lib/libsink.so.0.9
#30 0x00007ffff66b0b12 in KAsync::Private::Executor<void, QSharedPointer<QLocalSocket> >::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libsink.so.0.9
#31 0x00007ffff78f0f49 in KAsync::Private::Executor<void>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libkubeframework.so
#32 0x00007ffff78ca194 in KAsync::Job<void>::exec() () from /app/lib/libkubeframework.so
#33 0x00007ffff78f2b6f in KAsync::Private::Executor<void>::executeJobAndApply(std::function<KAsync::Job<void> ()> const&, KAsync::Future<void>&, std::integral_constant<bool, true>) () from /app/lib/libkubeframework.so
#34 0x00007ffff78f234f in KAsync::Private::Executor<void>::run(QSharedPointer<KAsync::Private::Execution> const&) () from /app/lib/libkubeframework.so
#35 0x00007ffff78f1b04 in KAsync::Private::Executor<void>::runExecution(KAsync::Future<void>*, QSharedPointer<KAsync::Private::Execution> const&, bool) () from /app/lib/libkubeframework.so
#36 0x00007ffff78f113a in KAsync::Private::Executor<void>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libkubeframework.so
#37 0x00007ffff78f0f49 in KAsync::Private::Executor<void>::exec(QSharedPointer<KAsync::Private::ExecutorBase> const&, QSharedPointer<KAsync::Private::ExecutionContext>) () from /app/lib/libkubeframework.so
#38 0x00007ffff78ca194 in KAsync::Job<void>::exec() () from /app/lib/libkubeframework.so
#39 0x00007ffff669614b in Sink::ResourceAccess::open() () from /app/lib/libsink.so.0.9
#40 0x00007ffff65f185e in Sink::Notifier::Notifier(Sink::Query const&)::{lambda(QSharedPointer<Sink::ApplicationDomain::SinkResource> const&)#1}::operator()(QSharedPointer<Sink::ApplicationDomain::SinkResource> const&) const ()
   from /app/lib/libsink.so.0.9
#41 0x00007ffff65f1eed in std::_Function_handler<void (QSharedPointer<Sink::ApplicationDomain::SinkResource> const&), Sink::Notifier::Notifier(Sink::Query const&)::{lambda(QSharedPointer<Sink::ApplicationDomain::SinkResource> const&)#1}>::_M_invoke(std::_Any_data const&, QSharedPointer<Sink::ApplicationDomain::SinkResource> const&) () from /app/lib/libsink.so.0.9
#42 0x00007ffff64bcfff in std::function<void (QSharedPointer<Sink::ApplicationDomain::SinkResource> const&)>::operator()(QSharedPointer<Sink::ApplicationDomain::SinkResource> const&) const () from /app/lib/libsink.so.0.9
#43 0x00007ffff64a187d in Sink::ResultEmitter<QSharedPointer<Sink::ApplicationDomain::SinkResource> >::add(QSharedPointer<Sink::ApplicationDomain::SinkResource> const&) () from /app/lib/libsink.so.0.9
#44 0x00007ffff6715828 in Sink::ResultProvider<QSharedPointer<Sink::ApplicationDomain::SinkResource> >::add(QSharedPointer<Sink::ApplicationDomain::SinkResource> const&) () from /app/lib/libsink.so.0.9
#45 0x00007ffff6759701 in LocalStorageQueryRunner<Sink::ApplicationDomain::SinkResource>::LocalStorageQueryRunner(Sink::Query const&, QByteArray const&, QByteArray const&, ConfigNotifier&, Sink::Log::Context const&)::{lambda()#2}::operator()() const ()
   from /app/lib/libsink.so.0.9
#46 0x00007ffff675ee1a in std::_Function_handler<void (), LocalStorageQueryRunner<Sink::ApplicationDomain::SinkResource>::LocalStorageQueryRunner(Sink::Query const&, QByteArray const&, QByteArray const&, ConfigNotifier&, Sink::Log::Context const&)::{lambda()#2}>::_M_invoke(std::_Any_data const&) () from /app/lib/libsink.so.0.9
#47 0x0000555555569ed0 in std::function<void ()>::operator()() const ()
#48 0x00007ffff66e7d8b in Sink::ResultProvider<QSharedPointer<Sink::ApplicationDomain::SinkResource> >::emitter()::{lambda()#2}::operator()() const () from /app/lib/libsink.so.0.9
#49 0x00007ffff67046df in std::_Function_handler<void (), Sink::ResultProvider<QSharedPointer<Sink::ApplicationDomain::SinkResource> >::emitter()::{lambda()#2}>::_M_invoke(std::_Any_data const&) () from /app/lib/libsink.so.0.9
#50 0x0000555555569ed0 in std::function<void ()>::operator()() const ()
#51 0x00007ffff641f096 in Sink::ResultEmitter<QSharedPointer<Sink::ApplicationDomain::SinkResource> >::fetch() () from /app/lib/libsink.so.0.9
#52 0x00007ffff65f1b8a in Sink::Notifier::Notifier(Sink::Query const&) () from /app/lib/libsink.so.0.9
#53 0x00007ffff79a6942 in SinkNotifier::SinkNotifier() () from /app/lib/libkubeframework.so
#54 0x00007ffff79a6a53 in SinkFabric::Private::Private() () from /app/lib/libkubeframework.so
#55 0x00007ffff79a23e1 in SinkFabric::SinkFabric() () from /app/lib/libkubeframework.so
#56 0x00007ffff79a24e4 in SinkFabric::instance() () from /app/lib/libkubeframework.so
#57 0x00007ffff79a11fe in Kube::Fabric::Bus::bringUpDeps() () from /app/lib/libkubeframework.so
#58 0x00007ffff79a1231 in Kube::Fabric::Bus::registerListener(Kube::Fabric::Listener*) () from /app/lib/libkubeframework.so
#59 0x00007ffff79a102b in Kube::Fabric::Listener::Listener(QObject*) () from /app/lib/libkubeframework.so
#60 0x00007fffdd137be3 in QQmlPrivate::QQmlElement<Kube::Fabric::Listener>::QQmlElement() () from /app/qml/org/kube/framework/libframeworkplugin.so
#61 0x00007fffdd137c25 in void QQmlPrivate::createInto<Kube::Fabric::Listener>(void*) () from /app/qml/org/kube/framework/libframeworkplugin.so
--Type <RET> for more, q to quit, c to continue without paging--
#62 0x00007ffff755f172 in QQmlType::create (this=this@entry=0x7fffffffd748, out=out@entry=0x7fffffffd740, memory=memory@entry=0x7fffffffd750, additionalMemory=additionalMemory@entry=144) at qml/qqmlmetatype.cpp:1043
#63 0x00007ffff75c3a45 in QQmlObjectCreator::createInstance (this=0x7fffffffdeb0, index=5, parent=0x555555848e50, isContextObject=<optimized out>) at qml/qqmlobjectcreator.cpp:1165
#64 0x00007ffff75c22a1 in QQmlObjectCreator::setPropertyBinding (this=0x7fffffffdeb0, bindingProperty=0x7fffd40e5628, binding=0x7fffd41d565c) at qml/qqmlobjectcreator.cpp:825
#65 0x00007ffff75c2a12 in QQmlObjectCreator::setupBindings (this=this@entry=0x7fffffffdeb0, applyDeferredBindings=applyDeferredBindings@entry=false) at qml/qqmlobjectcreator.cpp:777
#66 0x00007ffff75c3623 in QQmlObjectCreator::populateInstance (this=this@entry=0x7fffffffdeb0, index=-1, index@entry=0, instance=0x0, bindingTarget=0x0, valueTypeProperty=valueTypeProperty@entry=0x0) at qml/qqmlobjectcreator.cpp:1469
#67 0x00007ffff75c4211 in QQmlObjectCreator::createInstance (this=0x7fffffffdeb0, index=<optimized out>, parent=<optimized out>, isContextObject=<optimized out>) at qml/qqmlobjectcreator.cpp:1306
#68 0x00007ffff75c4ee0 in QQmlObjectCreator::create (this=this@entry=0x7fffffffdeb0, subComponentIndex=subComponentIndex@entry=-1, parent=parent@entry=0x0, interrupt=interrupt@entry=0x0) at qml/qqmlobjectcreator.cpp:203
#69 0x00007ffff75c3e63 in QQmlObjectCreator::createInstance (this=0x555555874e80, index=0, parent=0x0, isContextObject=<optimized out>) at qml/qqmlobjectcreator.cpp:1202
#70 0x00007ffff75c4ee0 in QQmlObjectCreator::create (this=this@entry=0x555555874e80, subComponentIndex=<optimized out>, parent=parent@entry=0x0, interrupt=interrupt@entry=0x0) at qml/qqmlobjectcreator.cpp:203
#71 0x00007ffff7546658 in QQmlComponentPrivate::beginCreate (this=0x5555555f3960, context=<optimized out>) at qml/qqmlcomponent.cpp:883
#72 0x00007ffff7544a3f in QQmlComponent::create (this=0x5555558a77a0, context=0x5555558b6d40) at qml/qqmlcomponent.cpp:795
#73 0x00007ffff75b50eb in QQmlApplicationEnginePrivate::finishLoad (this=this@entry=0x5555558d2c80, c=c@entry=0x5555558a77a0) at qml/qqmlapplicationengine.cpp:136
#74 0x00007ffff75b53ae in QQmlApplicationEnginePrivate::startLoad (this=0x5555558d2c80, url=..., data=..., dataFlag=dataFlag@entry=false) at qml/qqmlapplicationengine.cpp:120
#75 0x00007ffff75b53ed in QQmlApplicationEngine::load (this=<optimized out>, url=...) at /app/include/QtCore/qarraydata.h:257
#76 0x000055555555e3f5 in main ()
(gdb) Trace:   kolabnowCaldav.listener     : "No connections, shutting down."

Related Objects

Mentioned Here
T12398: API v2

FWIW; I have attempted but failed to reproduce this in a kasync testcase. I can reproduce it by starting latest kube with latest sink and latest kasync, and I can reproduce the crash in both flatpak (which I have now reverted to kasync 0.3.0), and a locally built kube.

Looks like some memory issue...could you try compiling with -fsanitize=address? I won't get to look into it properly before some time next week, sorry.

I'll try that. Next week is fine, no hurry.

Nothing very obvious from the address sanitizer so far, but I also haven't managed to run kube with it yet (only sinksh).

Ever since I have rebuilt a with -fsanitize=address I can no longer reproduce outside of the flatpak, so there's a chance that the fault is with flatpaks internal build chaching that somehow results in something that crashes (I can't think of a reasonable scenario, but who knows).
I'll try to completely rebuild the flatpak as well and see if this fixes the issue.

I have rebuilt the flatpak completely and can reproduce the crash. Also, I can reproduce the crash outside of the flatpak again, not sure what I did above.

tests/notificationtest in sink will produce a similar crash.

dvratil added a comment.EditedDec 17 2019, 2:34 PM

Hmm, I believe the bug might be in the Sink code. Here specifically, it's ResourceControl::flush():

The function registers two callbacks, both operating on the future reference. When the resource crashes (happens to me, I don't know if it's part of th etest), then sendFlushCommand fails, so future.setError is called (line 111). However, around the same time a notification is received with information about the crashed resource, invoking the lambda passed to registerHandler function, which then also calls future.setError() (line 98) - depending on which happens first, the future is finished at that point and completes the execution, which may delete the Future object, leaving the other lambda with a dangling reference....

I can probably solve the life-time of the future, however you are still calling setError twice on it, which should be undefined behavior - or, following what std::future does, would, well, not throw (because Qt), but might as well assert.

dvratil added a comment.EditedDec 17 2019, 2:39 PM

Generally, I'm wondering if I should deprecate this API and introduce a promise-future based API, where, instead of being passed a future from KAsync, the continuation would construct a Promise object internally a return a Future that KAsync would wait for....internally the continuation would own the Promise - it's closer to the common promise-future pattern and solves the lifetime issue, since the Promise is owned by the continuation, rather than by KAsync (which only holds the Future).

Hmm, I believe the bug might be in the Sink code. Here specifically, it's ResourceControl::flush():

The function registers two callbacks, both operating on the future reference. When the resource crashes (happens to me, I don't know if it's part of th etest), then sendFlushCommand fails, so future.setError is called (line 111). However, around the same time a notification is received with information about the crashed resource, invoking the lambda passed to registerHandler function, which then also calls future.setError() (line 98) - depending on which happens first, the future is finished at that point and completes the execution, which may delete the Future object, leaving the other lambda with a dangling reference....

I'll have to go look at some code to follow this theory, but given that I can clearly reproduce the crash with master but not with v0.3.0. Did you change anything with regards to lifetimes or so that would explain why this starts crashing now?

Asserting if any of the setError/setFinished functions are called multiple times is probably a good idea, as that seems to always point to a client bug I'd say. (setFinished is probably the right place for the assert as this is what "ends" the task)

Generally, I'm wondering if I should deprecate this API and introduce a promise-future based API, where, instead of being passed a future from KAsync, the continuation would construct a Promise object internally a return a Future that KAsync would wait for....internally the continuation would own the Promise - it's closer to the common promise-future pattern and solves the lifetime issue, since the Promise is owned by the continuation, rather than by KAsync (which only holds the Future).

This might result in a cleaner API indeed, I'm not sure it really solves anything related to this problem though. Some promise some place will have to be fulfilled exactly once.

dvratil added a comment.EditedDec 18 2019, 9:58 AM

It's not exactly the lifetime- it's a more deep-rooted issue with error handling.

The change that has happened is that historically Future<T> stored the value as a T* that was dynamically allocated when Future<T> was constructed and deleted when the Future<T> is destroyed. When Future<T>::setValue(foo) was called, the foo was copied into the object in T*. Now, instead Future<T> has a preallocated storage of size sizeof(T) that is not initialized by default. When setValue(foo) is called, foo is copy- or move-constructed in the storage, using placement new (see KAsync::Private::Storage<T> in future.h).

Now the way calling the error continuation is implemented is that the previous continuation's future's Error is passed in first, followed by previous continuation's future's value. This worked with the old version of Future, since Future<T> *always* held a valid default-constructed object of type T. Since my last change, that's not the case (it allows Future to hold non-default constructible and non-copyable types), so calling Future<T>::value() on a future that does not have any value (like one that has an error) returns garbage memory.

The proper fix here is, in my opinion, to assert when Future<T>::value() is called when Future<T>::setValue() was never called before. Second, the way the error continuation is invoked requires changes to prevent KAsync from attempting to pass the Future's value (which is wrong anyway) into the continuation when an error occurred. This *might* break API, so I need to look into it a bit more in detail.

In any case, the problem is known, now :)

dvratil added a comment.EditedDec 18 2019, 1:30 PM

There's no easy way how to fix this without reverting back to default-constructing T in Future<T>::Future().

The problem is that if I have a continuation like .then([](const KAsync::Error &e, int value), then if the previous continuation fails, I'll pass in the error, but I do not have the value to pass, because the previous future does not hold any (which is correct).

To solve this, I see three possible ways:

  1. disallow {Sync,Async,Job}ErrorContinuation in all KAsync functions except for .onError(), so that the code that until now was job.then([](const KAsync::Error &e, int value) { ... }) would look like job.onError([](const KAsync::Error &e) { ...}).then([](int value) { ... }); - question remains how to control flow, i.e. shoudl the second .then() continuation be called when error occured or should only onError be called and then the execution terminated?
  1. extend .then() (and everything else as well) to take two lambdas: one that takes the result of previous continuation, to be called when the previous continuation was OK, and one (optional) that takes KAsync::Error as an argument and is executed when previous continuation provides an error. This solves the problem described above as it can be explicitly decided by the programmer. Thus then([](const KAsync::Error &e, int value) { ... }) changes to .then([](int value) { ... }, [](const KAsync::Error &e) { ... }).
  1. modify *ErrorContinuation definitions to take a single argument: std::variant<KAsync::Error, T> to hold either a KAsync::Error or T. Solves the same issue as 2), but has the "problem" that std::variant is C++17... (which I would love to have anyway :D). It also makes it a slightly harder to work with non-copyable types T.

To be honest I think it makes most sense to revert back to default constructing T for the time being.

We can then devise a better plan for the future, which may as well be promise based and require various API changes (for which I would suggest we bump the major and don't do backwards compatibility).

OK, I have reverted the commit in master now. Sink tests pass again.

cmollekopf closed this task as Resolved.Dec 20 2019, 6:10 PM
cmollekopf claimed this task.

Thanks that did the trick.

I have tried and failed to write a test for the issue that I ran into;

+    auto future = KAsync::start<QSharedPointer<int>>(                                                                                                         
+        [](KAsync::Future<QSharedPointer<int>> &f) {                                                                                                          
+            f.setError({1, "error"});                                                                                                                         
+        })                                                                                                                                                    
+        .then([] (const KAsync::Error &error, const QSharedPointer<int> &ptr){                                                                                
+            if (error) {                                                                                                                                      
+                qDebug() << "We have an error";                                                                                                               
+            }                                                                                                                                                 
+        }).exec();                                                                                                                                            
+    QVERIFY(future.isFinished());

Works just fine for me, also before the patch was reverted.

For a next API iteration I have created T12398.