Details
-
Task
-
Resolution: Unresolved
-
P2: Important
-
None
-
Qt Creator 11.0.0-beta1
-
None
-
-
29f634a8c (master), f6e7dbd41 (11.0), d97d3f58a (11.0), 003b43aee (11.0), 664d40948 (11.0), 98026b29c (master), dda75153f (master), 35e03499f (master), 1da7f18fb (master), 2c0a59384 (master), 4c38f68d0 (master), e78d6bd3c (master), 01877b57d (master)
Description
Add a possibility to:
- Define "Timeout(int msecs)" item that could be inserted into a Group. Since now only Done and Error results of Task execution are possible, we should add a new Timeout type as next possible result.
- Resolution: Introduced FinishAllAndError workflow policy, and when combined with a Group containing the TimeoutTask, it serves for a real timeout (see also StopOnFinished). Added Group/TaskItem::withTimeout().
- Add new result type Canceled, issued when TaskTree::stop() was invoked, or when some task got aborted because of a failure of other task and stopped by a parent Group because of its workflow policy. May be diverged into two possibilities: CanceledByUser (TaskTree::stop() was executed) and CanceledByTaskTree (automatically canceled by TaskTree according to workflow policy). TODO: should be confronted with Group's workflow policy - maybe the latter needs to have more values?
- Resolution: Done
- Introduced DoneWith enum with Cancel value. No CanceledBy[User/TaskTree] differentiation for now.
- Introduced some behavioral changes in conjunction with WorkflowPolicy. Seems the new behavior is more sensible.
- Resolution: Done
- Define "Repeat(int count)" (alternatively: "Loop") item that could be inserted into a Group. This would repeat the execution of the whole group "count" times in a row. Optonally, it could take a std::function<bool()> argument so that the Group is repeated until the condition handler returns false. Useful e.g. for FileContainerInterator, where the exact number of iterations isn't known in advance. Challenges:
- Behave gracefully when combined with parallelLimit.
- Combine somehow with the task tree's progress info.
- Take care of the Storage elements inside the subgroups. See point 23.
- See
QTCREATORBUG-30081. - Resolution: Added simple Loop, Repeat and Forever elements. The container iterator, if still needed, may be added as a separate feature.
- Define "Delay(int msecs)" item that could be inserted into a Group. Similar to Timeout(int msecs), but this element could define its custom done handler.
- Resoultion: Introduced Timeout task
- Make it possible to start next (or nested) task only after the previous (parent) already started (or later, when it already replied it's ready). For this we would need to add TaskInterface::started() signal (or other, like: readyToStartSubTasks()). Nesting tasks in this case is better, since stopping it would preserve the right order automatically. TaskInterface::started() signal seems enough - other cases (when ready after task replied) are very specific to the particular task and providing a new adapter with delayed emission of started() signal would be enough (similar to how it's done in case of master ssh process). Whenever a parent task finishes, all children should be stopped automatically . When the last child finishes, should parent be automatically finished .
- Resolution: Introduced Barrier primitive
- Add fine-grained progress reporting for all task types. Maybe task setup handler could accept additional argument, similar to QPromise with regards to progress reporting?
- Add a "benchmark" group element that measures the group execution time and prints on debug output the relevant info on group done / error. The "benchmark" could take string arg to be included in printout.
- Resolution: Done. The benchmark comes with withLog() function - see point 8.
- Consider also the "log" group element that would log all setup/start/done/error events. Make it configurable? Make it possible to log recursively? Automatically log the tasking element name, like it's done with QTC_ASSERT?
- Resolution: Done. Added withLog() function that logs element start and end and prints the task's execution time.
- Unify done and error handlers (like Utils::Process unified finished() and errorOccurred() signals into one common done() signal):
- task's done and error handlers (leave just done handler with additional, not mandatory argument - may be a struct)
- OnGroupDone and OnGroupError handers (leave just OnGroupDone with additional, not mandatory argument - same as in case of task's done handler)
- TaskTree::done() and TaskTree::errorOccurred() signals (leave just done() signal with additional argument - same as in case of OnGroupDone handler).
- Rationale:
- It's often case that user code connects to both TaskTree::done() and TaskTree::errorOccurred() signals and the implementation is nearly the same.
- It's more clear that the task tree is already finished when there is only one done() signal. When having errorOccurred() in addition it's not clear that done() handler won't be called afterwards.
- When having just one done handler, make it possible to dynamically report the opposite result, e.g. report Done on Error or report Error on Done. This would work in a similar way do dynamic setup handler. Sometimes it's useful to let the handler decide about the ultimate result (e.g. in NetworkRequestTask, when request was OK, but it was e.g. redirected, and in this case we want to report an error). No return value would mean the originally reported value. The return value would be optional.
- Resolution: Done.
- Rename TaskItem into GroupItem. (Rename internal TaskNode and TaskContainer accordingly). Resolution: Done.
- Rationale: Not all classes derived from TaskItem are tasks, but the common denominator is that all may be placed inside a group: thus GroupItem sounds more appropriate.
- Rename TaskTree::setRoot() into setRecipe / setRunRecipe / setExecutionRecipe. Resolution: Done, renamed to TaskTree::setRecipe().
- The passed Group root element is a recipe with a full description on how the task tree should execute and handle finished tasks.
- We have already virtual methods / setters called like: deployRecipe, refreshRecipe, reloadRecipe
- Rename TaskTree into AsyncTree
- In fact, all tasks are asynchronous tasks (with a small exception to Sync element).
- Rename CustomTask into Task (or AsyncTask, when the previous point is done -> in this case the existing Async and AsyncTask should be renamed again).
- Rationale: Every final task is derived from CustomTask, so the Task name is general enough and more appropriate here. Note: don't confuse with ProjectExplorer::Task (and don't clash when both ProjectExplorer and Utils::Tasking namespaces are used together)
- The ProjectExplorer::Task may be a subject to rename, as it's rather a simple data container, not a task per se. The name "Task" in too general in this context and shouldn't be reserved for this specific usage. The ProjectExplorer::Task may be confused with Utils::TaskTree framework and one may think that both classes may work together - that's wrong impression.
- Make most of current GroupItem's subclasses final. Resolution: Done.
- Add some tests for triggering the appropriate runtime assert (e.g. putting 2 instances of OnGroupSetup handler into one Group, placing 2 instances of the same copy of Storage into the tree, etc...). Resolution: Done.
- Pimpl GroupItem, make it implicitly shared - for Qt adaptation puproses. Measure somehow the impact, though it should be probably neglected.
- Refactor GroupItem c'tors
- When stopping the tasks inside a Group, stop them in the reverse order
- Detect when one handler invokes the other one directly. May happen when e.g. Barrier is used (e.g. when Barrier is activated from task's start or end handler, what calls the other handler directly). Unwind properly in this case.
- It may also happen that the delayed Barrier or TimeoutTask is finished, when some done handler entered the nested event loop, e.g. while showing a message box. In this case we can't unwind properly.
- When one still running task causes the other task to finish synchronously, we probably can't detect such a case. And it may happen, that in this case the other finished task may cause the emitter to be destructed directly, because e.g. the result caused the other tasks in the group to be stopped immediately.
- Detecting a case when handlers are called from each other is easy, but there is no good default behavior on how to follow properly.
- Get rid of Storage subclass of GroupItem. Make GroupItem(const TreeStorageBase &storage) c'tor public. Rename TreeStorage<> into Storage<>. Rename internals accordingly. Resolution: Done.
- Rename TaskAction enum into SetupResult. Resolution: Done.
- Remove TASKING_DECLARE_TASK macro. Spell out the task alias manually instead. Resolution: Done.
- Implement Storage shadowing: make it possible to place the same Storage object in different groups of the same task tree. When placed in the same group, the repetition should be silently ignored. This will be useful for implementing the Repeat functionality (3rd point). However, this makes the onStorageSetup / onStorageDone handlers ambiguous. Resolution: Done. The handlers are to be called multiple times right now.
- Implement Storage in a thread-safe way. Make sure that when the same storage is used in 2 TaskTrees running in different threads at the same time - such a case is safe. This needs to map the pointer to the running QThread and to ensure the access is thread safe. Resolution: Done. Stress test added.
- Add List element that help in constructing Group content using initializer list. See https://codereview.qt-project.org/c/qt-creator/qt-creator/+/516143 for more details. Resolution: Done.
- As a consequence, since List may contain GroupItem elements now, the GroupItem name doesn't seem well fitted in this context.
- Rename StopOnDone -> StopOnSuccess, ContinueOnDone -> ContinueOnSuccess and StopWithDone -> StopWithSuccess in order to conform to the newly added OnDone and DoneResult enums. Resolution: Done.
- Replace recently added explicit List element (see point 25.) with additional GroupItem c'tors that create Lists implicitly.
- Add SignalAwaiter task, or combine this functionality inside Barrier. The task should finish whenever the connected signal is emitted.
- Resolution: BarrierTask may be used directly. See https://codereview.qt-project.org/c/qt-creator/qt-creator/+/529355.
- Change the naming convention of tasks, i.e. replace the Task suffix with Item suffix. E.g. ProcessTask -> ProcessItem, and so on. It will conform better to their base GroupItem class name.
- Provide a wrapper around std::unique_ptr<TaskTree>, e.g. TaskTreeRunner that would automatically handle releasing the ptr after done. See the suggestion here: https://codereview.qt-project.org/c/qt-creator/qt-creator/+/529244
- Investigate whether it's possible to have a LoopUntil element for a container with different subrecipes.
- See more discussion here: https://codereview.qt-project.org/c/qt-creator/qt-creator/+/530482.
- Most probably feasible with a nested TaskTreeTask...
- The issue with progress reporting, since it's not a LoopRepeat.
- Provide more values to DoneResult. The idea comes from implementing the Axivion's authorizationRecipe(), where it could look a bit more natural when having the following values:
- StopWithSuccess and StopWithError would stop executing all running tasks and skip not started tasks in the group containing the task.
- Continue when placed inside a loop would stop executing all running tasks and skip not started tasks for the current iteration.
- Break when placed inside a loop would stop executing all running tasks and skip not started tasks for all running iterations.
- Return would stop the whole tree
- Make it consistent with SetupResult values, especially for Group's setup result, as Group is seen by its parent as a single task.
- Add even more values, like:
- Stop this task with success / an error, let the other running tasks in the contained group finish, but don't execute anymore tasks in this group (skip not started). Meaningful for parallel mode.
- Make CallDoneIf a flag type. Add Cancel value. Make the default value of Success | Error | Cancel.
- Rename CallDoneIf into CallDoneOn
- Add a possibility to detect synchronous and asynchronous chain of invocations
- Add internal counter that is bumped on every return of control to the event loop. The same counter id between 2 calls means the synchronous chain of invocations.
- Resolution: Done. Added asyncCount() function.
- Add info to the log about the synchronous / asynchronous execution of the element. Reuse the asyncCount() from point 34. Resolution: Done.
- Unify the policy regarding calling the group / task done handler after SetupResult::StopWith[Success/Done]. Rationale: This causes confusion in more sophisticated recipes, like authorizationRecipe in Axivion plugin.
- Consider skipping calling group's done handler when group's setup handler returns SetupResult::StopWith[Success/Done]. Review all such usages in order to avoid regressions.
- Consider a property of a parallel group to invoke its tasks' done handlers in the same order as they are placed in the group.
- This would mean that when the task is finished and one of the preceding tasks is still running, postpone the task deletion and call its done handler just after the preceding task is finished.
- Check all existing task adapters whether this would work.
- Make it possible to pass DoneResult type directly as a done handler for custom task and for onGroupDone(). Resolution: Done.
- This will simplify the constructs where we want to unconditionally tweak the done result value, without the additional handling. Example: Barrier tasks in https://codereview.qt-project.org/c/qt-creator/qt-creator/+/529355.
- Reduce the number of workflow policies, as current 7 different are a bit confusing. Consider giving them better names.
- Add "withAccept()" method, similar to "withCancel()", that would prolong the execution of the task tree until the given signal is emitted.
- Add overloads to "withAccept()" and "withCancel()" that would accept directly the pointer to QObject and a signal name (instead of a function returning object-signal pair).
- Add a possibility to report an error with an optional error message. Something like Utils::Result.
- The error messages reported from different levels would accumulate on the stack.
- The onGroupDone() handler could take an additional arg with the current sub-stack errors accumulated. (maybe later?).
- The TaskTree::done() signal could pass the final stack.
- Consider removing SetupResult, as it might be described better with If condition.
- As a consequence, onGroupSetup could be replaced by Sync.
- Browse for usages and see whether the new approach is really better. The inconvenience might be the case when onGroupSetup returns all 3 different results from the body. Check it in practice.
- Refactor For's Then group.
- Currently, when we place a storage inside the Then group, it's initialized once, and it's common for all sequential / parallel executions.
- Consider instantiating the separate storage on each iteration instead. The original behavior may be achieved by placing the storage inside the outer group.
- Currently, when we place onGroupSetup / onGroupDone handlers inside the Then group, they are called just once, on first enter and last leave.
- Consider invoking them on each iteration instead. The original behavior may be achieved by placing them inside the outer group.
- For the above 2: see https://codereview.qt-project.org/c/qt-creator/qt-creator/+/606183 - the inner Group seems artificial.
- Keep the execution mode and workflow policy common for the Then group.
- Currently, when we place a storage inside the Then group, it's initialized once, and it's common for all sequential / parallel executions.
The 1. and 3. could serve for stress test implementation inside AutoTest plugin.
Side note: when having 1. and 3., a TrafficLight example (similar to state machine in Qt) could be implemented using TaskTree. Resolution: Done. See https://codereview.qt-project.org/c/qt-creator/qt-creator/+/529355.
Attachments
Issue Links
- mentioned in
-
Page Loading...
Gerrit Reviews
For Gerrit Dashboard: QTCREATORBUG-28741 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
598809,6 | TaskTree: Add a test for delayed deletion of task after nested loop | 15.0 | qt-creator/qt-creator | Status: NEW | 0 | 0 |