k_delayed_work_cancel documentation inconsistent with behavior

[pabigot]

zephyr/include/kernel.h

Line 3117 in c6d37eb

* @retval -EINVAL Work item is being processed or has completed its work.

inaccurately documents the behavior of k_delayed_work_cancel(). Specifically, if the work item has already completed the function will return zero, rather than -EINVAL. Assuming the states of a work item:

• never submitted
• delayed
• pending
• processing
• completed
• cancelled

the only time -EINVAL would be observed is in never-submitted, cancelled, or if it transitioned from pending to processing during the cancellation attempt.

The intent may have been to implement to the documentation; in that case the work_q value must be set to null after processing (which may be risky), and possibly other conditions need to be reviewed.

This had an impact in #22756 where it is not possible to determine whether a pending operation has been completed without maintaining an external flag.

[pabigot]

@joerchan

[joerchan]

Thanks.

For my use-case I think a function similar to k_work_pending for k_delayed_work would have been very useful. Between the timeout running out (k_delayed_work_remaining_get) and the timeout handler submitting (or not processed) it's no possible to check if the delayed work has been submitted, but not completed.

[Vudentz]

So I was with the right impression on this one and the documentation is wrong in one way or the other, the question is shall we change the implementation to reflect what is stated in the documentation? That would mean cancel would return an error if neither the timer or pending flag is set.

Something like this:

diff --git a/kernel/work_q.c b/kernel/work_q.c
index 7023f70f07..d9b259b50b 100644
--- a/kernel/work_q.c
+++ b/kernel/work_q.c
@@ -65,7 +65,11 @@ static int work_cancel(struct k_delayed_work *work)
                        return -EINVAL;
                }
        } else {
-               (void)z_abort_timeout(&work->timeout);
+               int err = z_abort_timeout(&work->timeout);
+
+               if (err) {
+                       return err;
+               }
        }
 
        /* Detach from workqueue */