Add a cap to the queue locks #435

aggarg · 2022-01-04T19:44:31Z

Description

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 . 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.

Test Steps

Created a test project and verified that the assert triggers without the change and does not trigger after that change.

Related Issue

#419

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

RichardBarry · 2022-01-04T23:26:48Z

queue.c

+        if( ( UBaseType_t ) ( cTxLock ) < uxNumberOfTasks )           \
+        {                                                             \
+            configASSERT( ( cTxLock ) != queueINT8_MAX );             \
+            ( pxQueue )->cTxLock = ( int8_t ) ( ( cTxLock ) + 1 );    \


Might be better to put the cast on the "1", rather than the result of adding a uint_8 with an integer.

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

* Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

codecov · 2022-01-05T00:18:30Z

Codecov Report

Merging #435 (1e5889e) into main (8e2dd5b) will increase coverage by 0.29%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #435      +/-   ##
==========================================
+ Coverage   92.15%   92.44%   +0.29%     
==========================================
  Files           4        4              
  Lines        1274     1270       -4     
  Branches      343      343              
==========================================
  Hits         1174     1174              
+ Misses         53       50       -3     
+ Partials       47       46       -1

Flag	Coverage Δ
unittests	`92.44% <100.00%> (+0.29%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
queue.c	`93.75% <100.00%> (+0.50%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e2dd5b...1e5889e. Read the comment docs.

This ensures that the coverage does not go down with the PR FreeRTOS/FreeRTOS-Kernel#435. Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

Add tests to cover FreeRTOS/FreeRTOS-Kernel#435 This ensures that the coverage does not go down with the PR FreeRTOS/FreeRTOS-Kernel#435. Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

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: FreeRTOS#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>

* Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

Add tests to cover FreeRTOS/FreeRTOS-Kernel#435 This ensures that the coverage does not go down with the PR FreeRTOS/FreeRTOS-Kernel#435. Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

* Create CloudFormation template for demo setup * Add CF_ prefix to CloudFormation-created resources to avoid collisions * Update lexicon.txt * Create initial python setup script * Create separate demo_cleanup.py file * Move setup items to DemoSetup folder * Add demo_config.h setup to the demo_setup.py script * Modify error logging on demo_setup.py * Add file cleanup to demo_cleanup.py * Rename convert_pem_to_der.py to convert_credentials_to_der.py * Adjust comment wording on demo_cleanup.py * added configUSE_TICKLESS_IDLE (#764) * Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 (#766) * Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com> * Add tests to cover FreeRTOS/FreeRTOS-Kernel#435 (#768) Add tests to cover FreeRTOS/FreeRTOS-Kernel#435 This ensures that the coverage does not go down with the PR FreeRTOS/FreeRTOS-Kernel#435. Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com> * Add tests to increase queue code coverage (#770) These tests cover the following portion in the queue code: static void prvUnlockQueue( Queue_t * const pxQueue ) { ... if( prvNotifyQueueSetContainer( pxQueue ) != pdFALSE ) { /* The queue is a member of a queue set, and posting to * the queue set caused a higher priority task to unblock. * A context switch is required. */ vTaskMissedYield(); } else { mtCOVERAGE_TEST_MARKER(); } ... } Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com> * Update FreeRTOS-Kernel submodule pointer (#771) Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com> * Add new names to spell check dictionary (#772) * Return error if invalid input detected in transport layer (Send/Recv) (#773) * return error if invalid input detected in transport layer * Create mqtt_pkcs11_demo_helpers for AWS demos (#769) * Create mqtt_pkcs11_demo_helpers by modifying mqtt_demo_helpers * Update formatting and variable naming * Fix multi-line parameter formatting * Update file headers to match latest release version * GCC/Rx100 Demo project files update to e2 Studio v8 (#776) * Upgrade GCC project files for e2 studio v7.8.0 in Demo/RX100-RSK_GCC_e2studio folder * Update Demo project file to e2 Studio v8 and remove the .bat file. * Update the choice of toolchain version. * Update the link in file header. Co-authored-by: NoMaY (a user of Japan.RenesasRulz.com) <NoMaY-jp@outlook.com> * Update FreeRTOS-Cellular-Interface submodule pointer (#775) * Update cellular sub-module pointer * Add more log in cellular_setup.c to indicate error * Adjust cellular transport timeout value for demo application * Add default cellular module specific config in cellular_config.h * Create separate demo_cleanup.py file * Move setup items to DemoSetup folder * Add demo_config.h setup to the demo_setup.py script * Modify error logging on demo_setup.py * Add file cleanup to demo_cleanup.py * Rename convert_pem_to_der.py to convert_credentials_to_der.py * Adjust comment wording on demo_cleanup.py * Adjust comment wording on demo_config.h * Format files and reduce code redundancy * Update lexicon.txt * Remove preconfigured fields from demo_config,h * Update convert_credentials_to_der.py Co-authored-by: Archit Gupta <71798289+archigup@users.noreply.github.com> * Make python files executable Co-authored-by: Joseph Julicher <jjulicher@mac.com> Co-authored-by: Gaurav-Aggarwal-AWS <33462878+aggarg@users.noreply.github.com> Co-authored-by: Ravishankar Bhagavandas <bhagavar@amazon.com> Co-authored-by: ActoryOu <jay2002824@gmail.com> Co-authored-by: Ming Yue <mingyue86010@gmail.com> Co-authored-by: NoMaY (a user of Japan.RenesasRulz.com) <NoMaY-jp@outlook.com> Co-authored-by: chinglee-iot <61685396+chinglee-iot@users.noreply.github.com> Co-authored-by: Archit Gupta <71798289+archigup@users.noreply.github.com>

* Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

Add tests to cover FreeRTOS/FreeRTOS-Kernel#435 This ensures that the coverage does not go down with the PR FreeRTOS/FreeRTOS-Kernel#435. Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

* Create CloudFormation template for demo setup * Add CF_ prefix to CloudFormation-created resources to avoid collisions * Update lexicon.txt * Create initial python setup script * Create separate demo_cleanup.py file * Move setup items to DemoSetup folder * Add demo_config.h setup to the demo_setup.py script * Modify error logging on demo_setup.py * Add file cleanup to demo_cleanup.py * Rename convert_pem_to_der.py to convert_credentials_to_der.py * Adjust comment wording on demo_cleanup.py * added configUSE_TICKLESS_IDLE (FreeRTOS#764) * Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 (FreeRTOS#766) * Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com> * Add tests to cover FreeRTOS/FreeRTOS-Kernel#435 (FreeRTOS#768) Add tests to cover FreeRTOS/FreeRTOS-Kernel#435 This ensures that the coverage does not go down with the PR FreeRTOS/FreeRTOS-Kernel#435. Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com> * Add tests to increase queue code coverage (FreeRTOS#770) These tests cover the following portion in the queue code: static void prvUnlockQueue( Queue_t * const pxQueue ) { ... if( prvNotifyQueueSetContainer( pxQueue ) != pdFALSE ) { /* The queue is a member of a queue set, and posting to * the queue set caused a higher priority task to unblock. * A context switch is required. */ vTaskMissedYield(); } else { mtCOVERAGE_TEST_MARKER(); } ... } Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com> * Update FreeRTOS-Kernel submodule pointer (FreeRTOS#771) Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com> * Add new names to spell check dictionary (FreeRTOS#772) * Return error if invalid input detected in transport layer (Send/Recv) (FreeRTOS#773) * return error if invalid input detected in transport layer * Create mqtt_pkcs11_demo_helpers for AWS demos (FreeRTOS#769) * Create mqtt_pkcs11_demo_helpers by modifying mqtt_demo_helpers * Update formatting and variable naming * Fix multi-line parameter formatting * Update file headers to match latest release version * GCC/Rx100 Demo project files update to e2 Studio v8 (FreeRTOS#776) * Upgrade GCC project files for e2 studio v7.8.0 in Demo/RX100-RSK_GCC_e2studio folder * Update Demo project file to e2 Studio v8 and remove the .bat file. * Update the choice of toolchain version. * Update the link in file header. Co-authored-by: NoMaY (a user of Japan.RenesasRulz.com) <NoMaY-jp@outlook.com> * Update FreeRTOS-Cellular-Interface submodule pointer (FreeRTOS#775) * Update cellular sub-module pointer * Add more log in cellular_setup.c to indicate error * Adjust cellular transport timeout value for demo application * Add default cellular module specific config in cellular_config.h * Create separate demo_cleanup.py file * Move setup items to DemoSetup folder * Add demo_config.h setup to the demo_setup.py script * Modify error logging on demo_setup.py * Add file cleanup to demo_cleanup.py * Rename convert_pem_to_der.py to convert_credentials_to_der.py * Adjust comment wording on demo_cleanup.py * Adjust comment wording on demo_config.h * Format files and reduce code redundancy * Update lexicon.txt * Remove preconfigured fields from demo_config,h * Update convert_credentials_to_der.py Co-authored-by: Archit Gupta <71798289+archigup@users.noreply.github.com> * Make python files executable Co-authored-by: Joseph Julicher <jjulicher@mac.com> Co-authored-by: Gaurav-Aggarwal-AWS <33462878+aggarg@users.noreply.github.com> Co-authored-by: Ravishankar Bhagavandas <bhagavar@amazon.com> Co-authored-by: ActoryOu <jay2002824@gmail.com> Co-authored-by: Ming Yue <mingyue86010@gmail.com> Co-authored-by: NoMaY (a user of Japan.RenesasRulz.com) <NoMaY-jp@outlook.com> Co-authored-by: chinglee-iot <61685396+chinglee-iot@users.noreply.github.com> Co-authored-by: Archit Gupta <71798289+archigup@users.noreply.github.com>

Add a cap to the queue locks

d3892fd

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

aggarg requested a review from a team as a code owner January 4, 2022 19:44

aggarg added 2 commits January 4, 2022 11:49

Add explicit cast

0e3042c

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

Fix formatting check

7afc0e8

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

aggarg added a commit to aggarg/FreeRTOS that referenced this pull request Jan 4, 2022

Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435

2cc985f

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

aggarg mentioned this pull request Jan 4, 2022

Fix tests needed for https://github.com/FreeRTOS/FreeRTOS-Kernel/pull/435 FreeRTOS/FreeRTOS#766

Merged

n9wxu previously approved these changes Jan 4, 2022

View reviewed changes

RichardBarry reviewed Jan 4, 2022

View reviewed changes

Move cast

d41a949

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

aggarg dismissed n9wxu’s stale review via d41a949 January 5, 2022 00:10

Add cast

e340b18

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

aggarg added a commit to FreeRTOS/FreeRTOS that referenced this pull request Jan 5, 2022

Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 (#766)

26dcb22

* Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

Fix formatting

1e5889e

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

aggarg added a commit to aggarg/FreeRTOS that referenced this pull request Jan 5, 2022

Add tests to cover FreeRTOS/FreeRTOS-Kernel#435

97d71c8

This ensures that the coverage does not go down with the PR FreeRTOS/FreeRTOS-Kernel#435. Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

aggarg mentioned this pull request Jan 5, 2022

Add tests to cover https://github.com/FreeRTOS/FreeRTOS-Kernel/pull/435 FreeRTOS/FreeRTOS#768

Merged

paulbartell approved these changes Jan 6, 2022

View reviewed changes

RichardBarry approved these changes Jan 6, 2022

View reviewed changes

aggarg merged commit becbb89 into FreeRTOS:main Jan 6, 2022

aggarg deleted the queue_lock branch January 6, 2022 05:14

aggarg mentioned this pull request Jan 6, 2022

[BUG] xQueue FromISR() APIs can cause an assertion based on RxLock and TxLock #419

Closed

johnrhen pushed a commit to johnrhen/FreeRTOS that referenced this pull request Jan 24, 2022

Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 (FreeRTOS#766)

59e2c05

* Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

Zangetsu112 pushed a commit to Zangetsu112/FreeRTOS-evpp that referenced this pull request Aug 18, 2025

Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 (FreeRTOS#766)

bb73d0a

* Fix tests needed for FreeRTOS/FreeRTOS-Kernel#435 Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add a cap to the queue locks #435

Add a cap to the queue locks #435

Uh oh!

aggarg commented Jan 4, 2022 •

edited by RichardBarry

Loading

Uh oh!

RichardBarry Jan 4, 2022

Uh oh!

codecov bot commented Jan 5, 2022 •

edited

Loading

Uh oh!

Uh oh!

Add a cap to the queue locks #435

Add a cap to the queue locks #435

Uh oh!

Conversation

aggarg commented Jan 4, 2022 • edited by RichardBarry Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Steps

Related Issue

Uh oh!

RichardBarry Jan 4, 2022

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

aggarg commented Jan 4, 2022 •

edited by RichardBarry

Loading

codecov bot commented Jan 5, 2022 •

edited

Loading