Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update documentation for the ConditionVariable API #8511

Merged
merged 5 commits into from Oct 27, 2018
Merged

Update documentation for the ConditionVariable API #8511

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c86ceb1
docs(api-condvar): Update documentation for the ConditionVariable API
Oct 23, 2018
8ddfdf5
Copy edit ConditionVariable.h
Oct 23, 2018
5cc5e47
future to present tense
Oct 25, 2018
c8f9759
Adding punctuation
Oct 25, 2018
98e6d53
Capitalization, header level adjustment
Oct 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
216 changes: 144 additions & 72 deletions rtos/ConditionVariable.h
View file
Open in desktop
@@ -1,5 +1,5 @@
/* mbed Microcontroller Library
* Copyright (c) 2017-2017 ARM Limited
/* Mbed Microcontroller Library
* Copyright (c) 2017-2018 ARM Limited
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -35,108 +35,146 @@ namespace rtos {

struct Waiter;

/** This class provides a safe way to wait for or send notifications of condition changes
/** The ConditionVariable class is a synchronization primitive that allows
* threads to wait until a particular condition occurs.
*
* This class is used in conjunction with a mutex to safely wait for or
* notify waiters of condition changes to a resource accessible by multiple
* Use the condition variable in conjunction with a mutex to safely wait for
* or notify waiters of condition changes to a resource accessible by multiple
* threads.
*
* # Defined behavior
* The thread that intends to wait on a ConditionVariable must:
* - Acquire a lock on a mutex.
* - Execute `wait`, `wait_for` or `wait_until`. While the thread is waiting,
* the mutex is unlocked.
* - When the condition variable has been notified, or in the case of `wait_for`
* and `wait_until` the timeout expires, the thread is awakened.
*
* The thread that intends to notify a ConditionVariable must:
* - Acquire a lock on the mutex used to construct the condition variable.
* - Execute `notify_one` or `notify_all` on the condition variable.
*
* #### Defined behavior
* - All threads waiting on the condition variable wake when
* ConditionVariable::notify_all is called.
* - If one or more threads are waiting on the condition variable at least
* one of them wakes when ConditionVariable::notify is called.
* `ConditionVariable::notify_all` is called.
* - At least one thread waiting on the condition variable wakes
* when `ConditionVariable::notify_one` is called.
* - While a thread is waiting for notification of a
* ConditionVariable, it releases the lock held on the mutex.
* - The ConditionVariable reacquires the mutex lock before exiting the wait
* function.
*
* # Undefined behavior
* - The thread which is unblocked on ConditionVariable::notify_one is
* #### Undefined behavior
* - The thread that is unblocked on `ConditionVariable::notify_one` is
* undefined if there are multiple waiters.
melwee01 marked this conversation as resolved.
Show resolved Hide resolved
* - The order which in which waiting threads acquire the condition variable's
* mutex after ConditionVariable::notify_all is called is undefined.
* - When ConditionVariable::notify_one or ConditionVariable::notify_all is
* called and there are one or more waiters and one or more threads attempting
* to acquire the condition variable's mutex the order in which the mutex is
* - Calling wait if the mutex is not locked by the current thread is undefined
* behavior.
* - The order in which waiting threads acquire the condition variable's
* mutex after `ConditionVariable::notify_all` is called is undefined.
* - When `ConditionVariable::notify_one` or `ConditionVariable::notify_all` is
* called and there are one or more waiters, and one or more threads
* attempting to acquire the condition variable's mutex, the order in which the mutex is
* acquired is undefined.
Copy link
Contributor

Choose a reason for hiding this comment

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

unspecified

Also spurious notifications below are unspecified, but don't seem to be able to put a comment there.

* - The behavior of ConditionVariable::wait and ConditionVariable::wait_for
* - The behavior of `ConditionVariable::wait` and `ConditionVariable::wait_for`
* is undefined if the condition variable's mutex is locked more than once by
* the calling thread.
* - Spurious notifications (not triggered by the application) can occur
* - Spurious notifications (not triggered by the application) can occur,
* and it is not defined when these occur.
*
* @note Synchronization level: Thread safe
*
* Example:
*
* @code
* #include "mbed.h"
*
* Mutex mutex;
* ConditionVariable cond(mutex);
* ConditionVariable cv(mutex);
*
* // These variables are protected by locking mutex
* uint32_t count = 0;
* // These variables are protected by locking mutex.
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that should be locking the mutex

* uint32_t work_count = 0;
* bool done = false;
*
* void worker_thread()
* {
* mutex.lock();
* do {
* printf("Worker: Count %lu\r\n", count);
* // Acquire lock on mutex before accessing protected variables and waiting.
* mutex.lock();
*
* // Wait for a condition to change
* cond.wait();
* while (done == false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good point on making this a while loop. do is pretty dangerous with a condition variable.

But == false or == true is bad style

Either done isn't a boolean, in which case it's actively dangerous, or it is a boolean, in which case it's just obfuscation.

if (done) or while (!done) on a boolean reads more like natural language.

* printf("Worker thread: Count: %lu\r\n", work_count);
*
* } while (!done);
* printf("Worker: Exiting\r\n");
* mutex.unlock();
* }
* // Wait for main thread to notify the condition variable.
* printf("Worker thread: Waiting\r\n");
* cv.wait();
* }
*
* int main() {
* Thread thread;
* thread.start(worker_thread);
* printf("Worker: Exiting\r\n");
*
* for (int i = 0; i < 5; i++) {
*
* mutex.lock();
* // Change count and notify waiters of this
* count++;
* printf("Main: Set count to %lu\r\n", count);
* cond.notify_all();
* mutex.unlock();
* // The condition variable acquires the lock when exiting the `wait` function.
* // Unlock mutex when exiting the thread.
* mutex.unlock();
* }
*
* wait(1.0);
* }
* int main()
* {
* Thread thread;
* thread.start(worker_thread);
*
* for (int i = 0; i < 5; i++) {
* // Acquire lock on mutex before modifying variables and notifying.
* mutex.lock();
* // Change done and notify waiters of this
* done = true;
* printf("Main: Set done\r\n");
* cond.notify_all();
*
* // Change count and notify waiters of this.
* work_count++;
* printf("Main thread: Set count to: %lu\r\n", work_count);
* printf("Main thread: Notifying worker thread\r\n");
* cv.notify_all();
*
* // Mutex must be unlocked before the worker thread can acquire it.
* mutex.unlock();
*
* thread.join();
* wait(1.0);
* }
*
* // Change done and notify waiters of this.
* mutex.lock();
* done = true;
* cv.notify_all();
* mutex.unlock();
*
* thread.join();
*
* printf("Main: Exiting\r\n");
* }
* @endcode
*/

class ConditionVariable : private mbed::NonCopyable<ConditionVariable> {
public:
/** Create and Initialize a ConditionVariable object
/** Create and initialize a ConditionVariable object.
*
* @note You cannot call this function from ISR context.
*/
ConditionVariable(Mutex &mutex);

/** Wait for a notification
/** Wait for a notification.
*
* Wait until a notification occurs.
* Wait causes the current thread to block until the condition variable
* receives a notification from another thread.
*
* @note - The thread calling this function must be the owner of the
* ConditionVariable's mutex and it must be locked exactly once
* @note - Spurious notifications can occur so the caller of this API
* should check to make sure the condition they are waiting on has
* been met
* ConditionVariable's mutex, and it must be locked exactly once.
*
* @note - Spurious notifications can occur, so the caller of this API
* should check to make sure the condition the caller is waiting on has
* been met.
*
* @note - The current thread releases the lock while inside the wait
kjbracey marked this conversation as resolved.
Show resolved Hide resolved
* function and reacquires it upon exiting the function.
*
* Example:
* @code
* mutex.lock();
*
* while (!condition_met) {
* cond.wait();
* }
Expand All @@ -150,16 +188,24 @@ class ConditionVariable : private mbed::NonCopyable<ConditionVariable> {
*/
void wait();

/** Wait for a notification until specified time
/** Wait for a notification until the specified time.
*
* Wait until causes the current thread to block until the condition
* variable is notified, or a specific time given by millisec parameter is
* reached.
*
* @param millisec absolute end time referenced to Kernel::get_ms_count()
* @return true if a timeout occurred, false otherwise.
* @param millisec Absolute end time referenced to `Kernel::get_ms_count()`
* @return True if a timeout occurred, false otherwise.
melwee01 marked this conversation as resolved.
Show resolved Hide resolved
*
* @note - The thread calling this function must be the owner of the
* ConditionVariable's mutex and it must be locked exactly once
* @note - Spurious notifications can occur so the caller of this API
* should check to make sure the condition they are waiting on has
* been met
* ConditionVariable's mutex, and it must be locked exactly once.
*
* @note - Spurious notifications can occur, so the caller of this API
* should check to make sure the condition the caller is waiting on has
* been met.
*
* @note - The current thread releases the lock while inside the wait
* function and reacquires it upon exiting the function.
*
* Example:
* @code
Expand All @@ -183,16 +229,24 @@ class ConditionVariable : private mbed::NonCopyable<ConditionVariable> {
*/
bool wait_until(uint64_t millisec);

/** Wait for a notification or timeout
/** Wait for a notification or timeout.
*
* @param millisec timeout value or osWaitForever in case of no time-out.
* @return true if a timeout occurred, false otherwise.
* `Wait for` causes the current thread to block until the condition
* variable receives a notification from another thread, or the timeout
* specified by the millisec parameter is reached.
*
* @param millisec Timeout value or osWaitForever in case of no timeout.
* @return True if a timeout occurred, false otherwise.
*
* @note - The thread calling this function must be the owner of the
* ConditionVariable's mutex and it must be locked exactly once
* @note - Spurious notifications can occur so the caller of this API
* should check to make sure the condition they are waiting on has
* been met
* ConditionVariable's mutex, and it must be locked exactly once.
*
* @note - Spurious notifications can occur, so the caller of this API
* should check to make sure the condition the caller is waiting on has
* been met.
*
* @note - The current thread releases the lock while inside the wait
* function and reacquire it upon exiting the function.
*
* Example:
* @code
Expand All @@ -218,26 +272,42 @@ class ConditionVariable : private mbed::NonCopyable<ConditionVariable> {

/** Notify one waiter on this condition variable that a condition changed.
*
* @note - The thread calling this function must be the owner of the ConditionVariable's mutex
* This function unblocks one of the threads waiting for the condition
* variable.
*
* @note - The thread calling this function must be the owner of the
* ConditionVariable's mutex.
*
* @note - The thread that is unblocked on ConditionVariable::notify_one is
* undefined if there are multiple waiters.
*
* @note You cannot call this function from ISR context.
*/
void notify_one();

/** Notify all waiters on this condition variable that a condition changed.
*
* @note - The thread calling this function must be the owner of the ConditionVariable's mutex
* This function unblocks all of the threads waiting for the condition
* variable.
*
* @note - The thread calling this function must be the owner of the
* ConditionVariable's mutex.
*
* @note - If there are one or more waiters and one or more threads
* attempting to acquire the condition variable's mutex the order in which
* the mutex is acquired is undefined.
*
* @note You cannot call this function from ISR context.
*/
void notify_all();

/** ConditionVariable destructor
/** ConditionVariable destructor.
*
* @note You cannot call this function from ISR context.
*/
~ConditionVariable();

#if !defined(DOXYGEN_ONLY)
protected:
struct Waiter {
Waiter();
Expand All @@ -247,10 +317,12 @@ class ConditionVariable : private mbed::NonCopyable<ConditionVariable> {
bool in_list;
};

private:
static void _add_wait_list(Waiter **wait_list, Waiter *waiter);
static void _remove_wait_list(Waiter **wait_list, Waiter *waiter);
Mutex &_mutex;
Waiter *_wait_list;
#endif // !defined(DOXYGEN_ONLY)
};

}
Expand Down