Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions commandsv3/src/main/java/org/wpilib/command3/Coroutine.java
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,8 @@ public boolean partialSuccess() {
* <p>This method does nothing if no commands were successfully forked.
*/
public void awaitCompletion() {
for (Command command : m_forkedCommands) {
if (m_scheduler.isRunning(command)) {
Coroutine.this.yield();
}
while (m_forkedCommands.stream().anyMatch(m_scheduler::isRunning)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop tracking a fork entry after its original run ends

When one forked command completes while another remains active, this loop keeps the completed command in m_forkedCommands; if that same Command instance is subsequently scheduled again by a trigger, default binding, or direct scheduler call, isRunning() treats the new execution as part of the old fork. The parent then waits for this unrelated run and can hang indefinitely if it is persistent, contrary to the documented promise that completed forked commands are not rescheduled or awaited again. Track the original scheduling lifecycle, or permanently retire each entry once its forked execution ends.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SamCarlberg curious your thoughts on this—if this is a real concern, we could possibly track the ids of the originally scheduled commands:

    public void awaitCompletion() {
      Set<Integer> ids = m_forkedCommands.stream().map(Scheduler.getDefault()::runId).filter(x -> x > 0).collect(Collectors.toSet());
      Predicate<Command> isCommandStillRunning = command -> ids.contains(Scheduler.getDefault().runId(command));
      while (m_forkedCommands.stream().anyMatch(isCommandStillRunning)) {
        Coroutine.this.yield();
      }
    }

But I could also be overthinking this

Coroutine.this.yield();
}
}

Expand Down
Loading