patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 7 Packages Projects Releases Wiki Activity

work_q: Correctly clear pending flag in delayed work queue, update docs

Browse source 
As discovered in https://github.com/zephyrproject-rtos/zephyr/issues/5952

...a duplicate call to k_delayed_work_submit_to_queue() on a work item
whose timeout had expired but which had not yet executed (i.e. it was
pending in the queue for the active work queue thread) would fail,
because the cancellation step wouldn't clear the PENDING bit, causing
the resubmission to see the object in an invalid state.  Trivially
fixed by adding a bit clear.

It also turns out that the behavior of the code doesn't match the
docs, which state that a PENDING work item is not supposed to be
cancelled at all.  Fix the docs to remove that.

And on yet further review, it turns out that there's no way to make a
test like the one in the linked bug threadsafe.  The work queue does
no synchronization by design, so if the user code does no external
synchronization it might very well clobber the running handler.  Added
a sentence to the docs to reflect this gotcha.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This commit is contained in:
Andy Ross 2018-02-13 12:13:25 -08:00 committed by Anas Nashif
parent 771dfb0d13
commit 03c1d28e6e
2 changed files with 6 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
include/kernel.h
View file

@ -2306,12 +2306,11 @@ extern void k_delayed_work_init(struct k_delayed_work *work,
* the workqueue and becomes pending.
*
* Submitting a previously submitted delayed work item that is still
* counting down cancels the existing submission and restarts the countdown
* using the new delay. If the work item is currently pending on the
* workqueue's queue because the countdown has completed it is too late to
* resubmit the item, and resubmission fails without impacting the work item.
* If the work item has already been processed, or is currently being processed,
* its work is considered complete and the work item can be resubmitted.
* counting down cancels the existing submission and restarts the
* countdown using the new delay. Note that this behavior is
* inherently subject to race conditions with the pre-existing
* timeouts and work queue, so care must be taken to synchronize such
* resubmissions externally.
*
* @warning
* A delayed work item must not be modified until it has been processed

1
kernel/work_q.c
View file

@ -137,6 +137,7 @@ int k_delayed_work_cancel(struct k_delayed_work *work)
/* Detach from workqueue */
work->work_q = NULL;
atomic_clear_bit(work->work.flags, K_WORK_STATE_PENDING);
irq_unlock(key);
return 0;