Code review comment for lp:~xavi-garcia-mena/keeper/task-state-manager

Michi,

Yes, I think you're misunderstanding. What I'm saying is because we're using `delete o' instead of `o.deleteLater()' in the QLocalSocket's shared_ptr, we're setting up a case where it can be destroyed in the middle of its bytesWritten signal.

1. keeper calls create_file(), gets an uploader
2. uploader's ctor creates a QLocalSocket smart ptr (socket refcount init to 1)
3. keeper holds onto the uploader (uploader refcount init to 1)
4. keeper calls uploader::socket(), keeps a ref (socket refcount inc to 2), subscribes to bytesWritten
(work happens)
5. keeper gets a socket::bytesWritten signal and calls its update-task-status method

two paths here:

6a. if keeper's got an error flag set on this task, eg error between the backup helper and keeper, we set the task as in error and reset state, including our socket smart_ptr (socket refcount dec to 1) and our uploader smart_ptr (uploader refcount dec to 0, socket refcount dec to 0) so we've called "delete QLocalSocket" in the middle of its bytesWritten callback

(less sure about this, branch, steps to trigger same bug here depend on how Qt signals work)
6b. if no error flag but we've written all we planned to, keeper calls finish_upload()
7b. UploaderImpl::finish_upload() calls disconnectFromServer() in caller thread (ie, we're still in the QLocalSocket::bytesWritten slot wrt stack unwinding)
8b. disconnectFromServer() -> readChannelFinished signal -> uploader's on_read_channel_finished() slot -> do_finish() -> finalize() -> make_ready_future(qf_, file) -> keeper's QFutureListener -> keeper's ``file ready'' handler
9b. keeper says "we're done, clear the state", including our socket reference (socket refcount dec to 1) and our uploader (uploader refcount dec to 0, socket refcount dec to 0), so again we've called 'delete QLocalSocket" in the middle of its bytesWritten callback

On top of that, keeper has its own bug in branch b: it's letting the uploader refcount dec to 0 while still having a QFutureWatcher has a dangling reference to the now-destroyed uploader's QFuture.