Add a cap to the queue locks (#435)
Add a cap to the queue locks
cRxLock and cTxLock members of the queue data structure count the
number items received and sent to the queue while the queue was locked.
These are later used to unblock tasks waiting on the queue when the
queue is unlocked. The data type of these members is int8_t and this can
trigger overflow check assert if an ISR continuously reads/writes to the
queue in a loop as reported in this issue: https://github.com/FreeRTOS/FreeRTOS-Kernel/issues/419.
Note due to the length of the operation is it not recommended to write to
the queue that many times from an ISR - stream buffers are a better option,
or alternatively, defer the operation to a task by just having the ISR send a
direct to task notification to unblock the task.
This PR caps the values of the cRxLock and cTxLock to the number of tasks in
the system because we cannot unblocks more tasks than there are in the system.
Note that the same assert could still be triggered is the application creates more
than 127 tasks.
Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
diff --git a/queue.c b/queue.c
index f3899ae..718afe6 100644
--- a/queue.c
+++ b/queue.c
@@ -261,6 +261,36 @@
} \
} \
taskEXIT_CRITICAL()
+
+/*
+ * Macro to increment cTxLock member of the queue data structure. It is
+ * capped at the number of tasks in the system as we cannot unblock more
+ * tasks than the number of tasks in the system.
+ */
+#define prvIncrementQueueTxLock( pxQueue, cTxLock ) \
+ { \
+ const UBaseType_t uxNumberOfTasks = uxTaskGetNumberOfTasks(); \
+ if( ( UBaseType_t ) ( cTxLock ) < uxNumberOfTasks ) \
+ { \
+ configASSERT( ( cTxLock ) != queueINT8_MAX ); \
+ ( pxQueue )->cTxLock = ( int8_t ) ( ( cTxLock ) + ( int8_t ) 1 ); \
+ } \
+ }
+
+/*
+ * Macro to increment cRxLock member of the queue data structure. It is
+ * capped at the number of tasks in the system as we cannot unblock more
+ * tasks than the number of tasks in the system.
+ */
+#define prvIncrementQueueRxLock( pxQueue, cRxLock ) \
+ { \
+ const UBaseType_t uxNumberOfTasks = uxTaskGetNumberOfTasks(); \
+ if( ( UBaseType_t ) ( cRxLock ) < uxNumberOfTasks ) \
+ { \
+ configASSERT( ( cRxLock ) != queueINT8_MAX ); \
+ ( pxQueue )->cRxLock = ( int8_t ) ( ( cRxLock ) + ( int8_t ) 1 ); \
+ } \
+ }
/*-----------------------------------------------------------*/
BaseType_t xQueueGenericReset( QueueHandle_t xQueue,
@@ -1162,9 +1192,7 @@
{
/* Increment the lock count so the task that unlocks the queue
* knows that data was posted while it was locked. */
- configASSERT( cTxLock != queueINT8_MAX );
-
- pxQueue->cTxLock = ( int8_t ) ( cTxLock + 1 );
+ prvIncrementQueueTxLock( pxQueue, cTxLock );
}
xReturn = pdPASS;
@@ -1330,9 +1358,7 @@
{
/* Increment the lock count so the task that unlocks the queue
* knows that data was posted while it was locked. */
- configASSERT( cTxLock != queueINT8_MAX );
-
- pxQueue->cTxLock = ( int8_t ) ( cTxLock + 1 );
+ prvIncrementQueueTxLock( pxQueue, cTxLock );
}
xReturn = pdPASS;
@@ -1938,9 +1964,7 @@
{
/* Increment the lock count so the task that unlocks the queue
* knows that data was removed while it was locked. */
- configASSERT( cRxLock != queueINT8_MAX );
-
- pxQueue->cRxLock = ( int8_t ) ( cRxLock + 1 );
+ prvIncrementQueueRxLock( pxQueue, cRxLock );
}
xReturn = pdPASS;
@@ -3058,9 +3082,7 @@
}
else
{
- configASSERT( cTxLock != queueINT8_MAX );
-
- pxQueueSetContainer->cTxLock = ( int8_t ) ( cTxLock + 1 );
+ prvIncrementQueueTxLock( pxQueueSetContainer, cTxLock );
}
}
else
