Skip to content

Commit

Permalink
locking: Simplify maintenance / debugging /documentation
Browse files Browse the repository at this point in the history 
Update the Public API equivalent of the libcoap functions that need to be
called when locked with a name suffix of _lkd.

Move matching Public API functions out of coap_threadsafe.c to be coded
just ahead of the newly renamed _lkd function.

Properly define the _lkd function in the appropriate _internal.h file for
documentation, removing the #defines from coap_threadsafe_internal.h.

Tag all the Public API functions that have a _lkd function with COAP_API.
COAP_API can then be defined as to what should be done for that function.

Check libcoap library does not call the original Public API function when
it should be calling the _lkd function by marking the Public API not _lkd
functions as deprecated during the libcoap library build.
[Done by the new COAP_API being defined as __attribute__((deprecated)).]

Work in progress for all the functions that need to be multi-thread safe.
  • Loading branch information
@mrdeep1
mrdeep1 committed May 24, 2024
1 parent 948697e commit cb20c48
Show file tree
Hide file tree
Showing 37 changed files with 1,149 additions and 920 deletions.
11 changes: 0 additions & 11 deletions examples/lwip/client-coap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,6 @@

#include "coap_config.h"

#if COAP_THREAD_SAFE
/*
* Unfortunately, this needs to be set so that locking mapping of coap_
* functions does not take place in this file. coap.h includes coap_mem.h which
* includes lwip headers (lwippools.h) which includes coap_internal.h which
* includes coap_threadsafe_internal.h which does the mapping unless
* COAP_THREAD_IGNORE_LOCKED_MAPPING is set.
*/
#define COAP_THREAD_IGNORE_LOCKED_MAPPING
#endif

#include <coap3/coap.h>
#include <sys/types.h>
#include <sys/socket.h>
Expand Down
11 changes: 0 additions & 11 deletions examples/lwip/server-coap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,6 @@

#include "coap_config.h"

#if COAP_THREAD_SAFE
/*
* Unfortunately, this needs to be set so that locking mapping of coap_
* functions does not take place in this file. coap.h includes coap_mem.h which
* includes lwip headers (lwippools.h) which includes coap_internal.h which
* includes coap_threadsafe_internal.h which does the mapping unless
* COAP_THREAD_IGNORE_LOCKED_MAPPING is set.
*/
#define COAP_THREAD_IGNORE_LOCKED_MAPPING
#endif

#include <coap3/coap.h>
#include "server-coap.h"

Expand Down
6 changes: 3 additions & 3 deletions include/coap3/coap_block.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ COAP_API int coap_add_data_large_response(coap_resource_t *resource,
* @param context The coap_context_t object.
* @param block_mode Zero or more COAP_BLOCK_ or'd options
*/
void coap_context_set_block_mode(coap_context_t *context,
uint32_t block_mode);
COAP_API void coap_context_set_block_mode(coap_context_t *context,
uint32_t block_mode);

/**
* Set the context level maximum block size that the server supports when sending
Expand All @@ -440,7 +440,7 @@ void coap_context_set_block_mode(coap_context_t *context,
* @param max_block_size The maximum block size a server supports. Can be 0
* (reset), or must be 16, 32, 64, 128, 256, 512 or 1024.
*/
int coap_context_set_max_block_size(coap_context_t *context, size_t max_block_size);
COAP_API int coap_context_set_max_block_size(coap_context_t *context, size_t max_block_size);

/**@}*/

Expand Down
39 changes: 39 additions & 0 deletions include/coap3/coap_block_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,45 @@ void coap_check_code_lg_xmit(const coap_session_t *session,
const coap_resource_t *resource,
const coap_string_t *query);

/**
* Set the context level CoAP block handling bits for handling RFC7959.
* These bits flow down to a session when a session is created and if the peer
* does not support something, an appropriate bit may get disabled in the
* session block_mode.
* The session block_mode then flows down into coap_crcv_t or coap_srcv_t where
* again an appropriate bit may get disabled.
*
* Note: This function must be called before the session is set up.
*
* Note: COAP_BLOCK_USE_LIBCOAP must be set if libcoap is to do all the
* block tracking and requesting, otherwise the application will have to do
* all of this work (the default if coap_context_set_block_mode() is not
* called).
*
* @param context The coap_context_t object.
* @param block_mode Zero or more COAP_BLOCK_ or'd options
*/
void coap_context_set_block_mode_lkd(coap_context_t *context,
uint32_t block_mode);

/**
* Set the context level maximum block size that the server supports when sending
* or receiving packets with Block1 or Block2 options.
* This maximum block size flows down to a session when a session is created.
*
* Note: This function must be called before the session is set up.
*
* Note: This function must be called before the session is set up.
*
* Note: COAP_BLOCK_USE_LIBCOAP must be set using coap_context_set_block_mode()
* if libcoap is to do this work.
*
* @param context The coap_context_t object.
* @param max_block_size The maximum block size a server supports. Can be 0
* (reset), or must be 16, 32, 64, 128, 256, 512 or 1024.
*/
int coap_context_set_max_block_size_lkd(coap_context_t *context, size_t max_block_size);

#if COAP_CLIENT_SUPPORT
/**
* Associates given data with the @p pdu that is passed as second parameter.
Expand Down
4 changes: 2 additions & 2 deletions include/coap3/coap_layers_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ typedef ssize_t (*coap_layer_write_t)(coap_session_t *session,
* session->lfunc[_this_layer_].l_establish(session)
* (or done at any point when layer is established).
* If the establishment of a layer fails, then
* coap_session_disconnected(session, COAP_NACK_xxx_LAYER_FAILED) must be
* called.
* coap_session_disconnected_lkd(session, COAP_NACK_xxx_LAYER_FAILED) must
* be called.
*
* @param session Session being established
*/
Expand Down
235 changes: 0 additions & 235 deletions include/coap3/coap_mutex_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,239 +161,4 @@ extern coap_mutex_t m_persist_add;

#endif /* COAP_CONSTRAINED_STACK */

/*
* Support thread safe access into libcoap
*
* Locking at different component levels (i.e context and session) is
* problematic in that coap_process_io() needs to lock the context as
* it scans for all the sessions and then could lock the session being
* processed as well - but context needs to remain locked as a list is
* being scanned.
*
* Then if the session process needs to update context ( e.g. delayqueue),
* context needs to be locked. So, if coap_send() is done on a session,
* it has to be locked, but a retransmission of a PDU by coap_process_io()
* has the context already locked.
*
* So the initial support for thread safe is done at the context level.
*
* New model, simplifying debugging and better documentation.
*
* Any public API call needs to potentially lock context, as there may be
* multiple contexts. If a public API needs thread safe protection, the
* coap_X() function locks the context lock, calls the coap_X_lkd() function
* that does all the work and on return unlocks the context before returning
* to the caller of coap_X().
*
* Any internal libcoap calls that are to the public API coap_X() must call
* coap_X_lkd() if the calling code is already locked.
*
* Old, equivalent model
*
* Any public API call needs to potentially lock context, as there may be
* multiple contexts. If a public API needs thread safe protection, a
* locking wrapper for coap_X() is added to src/coap_threadsafe.c which then
* calls the coap_X_locked() function of coap_X() having locked context.
*
* Then an entry is added to include/coap3/coap_threadsafe_internal.h to map
* all the coap_X() definitions and calls within the libcoap code to
* coap_X_locked() (with the exception of src/coap_threadsafe.c).
*
* A second entry is added to include/coap3/coap_threadsafe_internal.h which
* defines the coap_X_locked() function header.
*
* Any call-back into app space must be done by using the coap_lock_callback()
* (or coap_lock_callback_ret()) wrapper.
*
* Note:
* libcoap may call a handler, which may in turn call into libcoap, which may
* then call a handler. context will remain locked thoughout this process.
*
* Any wait on select() or equivalent when a thread is waiting on an event
* must be preceded by unlock context, and then context re-locked after
* return;
*
* To check for recursive deadlocks, COAP_THREAD_RECURSIVE_CHECK needs to be
* defined.
*
* If thread safe is not enabled, then coap_threadsafe.c and
* coap_threadsafe_internal.h do nothing.
*/
#if COAP_THREAD_SAFE
# if COAP_THREAD_RECURSIVE_CHECK

/*
* Locking, with deadlock detection
*/
typedef struct coap_lock_t {
coap_mutex_t mutex;
coap_thread_pid_t pid;
coap_thread_pid_t freeing_pid;
const char *lock_file;
unsigned int lock_line;
unsigned int unlock_line;
const char *unlock_file;
const char *callback_file;
unsigned int callback_line;
unsigned int being_freed;
unsigned int in_callback;
unsigned int lock_count;
} coap_lock_t;

void coap_lock_unlock_func(coap_lock_t *lock, const char *file, int line);
int coap_lock_lock_func(coap_lock_t *lock, const char *file, int line);

#define coap_lock_lock(s,failed) do { \
assert(s); \
if (!coap_lock_lock_func(&(s)->lock, __FILE__, __LINE__)) { \
failed; \
} \
} while (0)

#define coap_lock_unlock(s) do { \
assert(s); \
coap_lock_unlock_func(&(s)->lock, __FILE__, __LINE__); \
} while (0)

#define coap_lock_callback(s,func) do { \
coap_lock_check_locked(s); \
(s)->lock.in_callback++; \
(s)->lock.callback_file = __FILE__; \
(s)->lock.callback_line = __LINE__; \
func; \
(s)->lock.in_callback--; \
} while (0)

#define coap_lock_callback_ret(r,s,func) do { \
coap_lock_check_locked(s); \
(s)->lock.in_callback++; \
(s)->lock.callback_file = __FILE__; \
(s)->lock.callback_line = __LINE__; \
r = func; \
(s)->lock.in_callback--; \
} while (0)

#define coap_lock_callback_release(s,func,fail) do { \
coap_lock_check_locked(s); \
coap_lock_unlock(s); \
func; \
coap_lock_lock(s,fail); \
} while (0)

#define coap_lock_callback_ret_release(r,s,func,fail) do { \
coap_lock_check_locked(s); \
coap_lock_unlock(s); \
r = func; \
coap_lock_lock(s,fail); \
} while (0)

# else /* ! COAP_THREAD_RECURSIVE_CHECK */

/*
* Locking, but no deadlock detection
*/
typedef struct coap_lock_t {
coap_mutex_t mutex;
coap_thread_pid_t pid;
coap_thread_pid_t freeing_pid;
uint32_t being_freed;
uint32_t in_callback;
volatile uint32_t lock_count;
} coap_lock_t;

void coap_lock_unlock_func(coap_lock_t *lock);
int coap_lock_lock_func(coap_lock_t *lock);

#define coap_lock_lock(s,failed) do { \
assert(s); \
if (!coap_lock_lock_func(&(s)->lock)) { \
failed; \
} \
} while (0)

#define coap_lock_unlock(s) do { \
assert(s); \
coap_lock_unlock_func(&(s)->lock); \
} while (0)

#define coap_lock_callback(s,func) do { \
coap_lock_check_locked(s); \
(s)->lock.in_callback++; \
func; \
(s)->lock.in_callback--; \
} while (0)

#define coap_lock_callback_ret(r,s,func) do { \
coap_lock_check_locked(s); \
(s)->lock.in_callback++; \
r = func; \
(s)->lock.in_callback--; \
} while (0)

#define coap_lock_callback_release(s,func,fail) do { \
coap_lock_check_locked(s); \
coap_lock_unlock(s); \
func; \
coap_lock_lock(s,fail); \
} while (0)

#define coap_lock_callback_ret_release(r,s,func,fail) do { \
coap_lock_check_locked(s); \
coap_lock_unlock(s); \
r = func; \
coap_lock_lock(s,fail); \
} while (0)

# endif /* ! COAP_THREAD_RECURSIVE_CHECK */

#define coap_lock_init(s) do { \
assert(s); \
memset(&((s)->lock), 0, sizeof((s)->lock)); \
coap_mutex_init(&(s)->lock.mutex); \
} while (0)

#define coap_lock_being_freed(s,failed) do { \
coap_lock_lock(s,failed); \
(s)->lock.being_freed = 1; \
(s)->lock.freeing_pid = coap_thread_pid; \
coap_lock_unlock(s); \
} while (0)

#define coap_lock_check_locked(s) do { \
assert((s) && \
coap_thread_pid == ((s)->lock.being_freed ? (s)->lock.freeing_pid : \
(s)->lock.pid)); \
} while (0)

#define coap_lock_invert(s,func,f) do { \
coap_lock_check_locked(s); \
if (!(s)->lock.being_freed) { \
coap_lock_unlock(s); \
func; \
coap_lock_lock(s,f); \
} else { \
func; \
} \
} while (0)

#else /* ! COAP_THREAD_SAFE */

/*
* No locking - single thread
*/
typedef coap_mutex_t coap_lock_t;

#define coap_lock_lock(s,failed)
#define coap_lock_unlock(s)
#define coap_lock_init(s)
#define coap_lock_being_freed(s,failed)
#define coap_lock_check_locked(s) {}
#define coap_lock_callback(s,func) func
#define coap_lock_callback_ret(r,s,func) ret = func
#define coap_lock_callback_release(s,func,fail) func
#define coap_lock_callback_ret_release(r,s,func,fail) ret = func
#define coap_lock_invert(s,func,f) func

#endif /* ! COAP_THREAD_SAFE */

#endif /* COAP_MUTEX_INTERNAL_H_ */
Loading

0 comments on commit cb20c48

Please sign in to comment.