CriticalSectionLock class improvement #5621

maciejbocianski · 2017-11-30T14:46:23Z

Description

Change CriticalSectionLock member functions void lock/unlock() to static ones
static void lock/unlock() to allow more meaningful lock/unlock usage

Status

READY

Migrations

Although the class API is changed, usage is not affected since class member access syntax is allowed for static functions

YES

Related PRs

#5420

0xc0170 · 2017-12-01T10:28:40Z

@tommikas Please can you look at this failure, I found at least one more PR that looks like a network failure (this should not cause it as it does not change any code behavior).

maciejbocianski · 2017-12-04T14:31:58Z

Guys

What do you think about adding is_locked function to CriticalSection class to cover full core_util_critical_section interface?
(for more details see #5346)

static bool is_locked()
{
    return core_util_in_critical_section();
}

0xc0170 · 2017-12-04T15:08:22Z

If HAL defines it (this however is in progress, so this can be added once critical section improvements get in, as an extension , or?) then it would be good to have.

deepikabhavnani

Looks good 👍

c1728p9 · 2017-12-11T17:46:25Z

platform/CriticalSectionLock.h

     */
+    MBED_DEPRECATED_SINCE("mbed-os-5.7",
+            "This function is inconsistent with RAII and is being removed in the future.")
    void lock()


I think Sam has concerns about deprecating this function.

As I remember Sam had concerns about deprecating whole class

c1728p9

Looks good to me aside from Sam's concerns about the deprecation.

sg-

deprecation of lock/unlock is ok with replacement by static enable/disable members. make sure the note specifies how to use this

maciejbocianski · 2017-12-18T09:54:38Z

requested changes applied @sg-

sg- · 2017-12-18T16:09:16Z

platform/CriticalSectionLock.h

@@ -33,14 +33,25 @@ namespace mbed {
  * Usage:
  * @code
  *
-  * void f() {
+  * // RAII style usage
+  * void foo() {


Lets make this a real example, not foo and bar, etc

sg- · 2017-12-18T16:09:31Z

platform/CriticalSectionLock.h

  *     // some code here
  *     {
  *         CriticalSectionLock lock;
  *         // Code in this block will run with interrupts disabled
  *     }
  *     // interrupts will be restored to their previous state
  * }
+  *
+  * // free locking usage
+  * void bar() {


Lets make this a real example, not foo and bar, etc

sg- · 2017-12-18T16:10:42Z

platform/CriticalSectionLock.h

     */
+    MBED_DEPRECATED_SINCE("mbed-os-5.8",
+            "This function is inconsistent with RAII and is being removed in the future.")


The deprecated note should have the name syntax of what replaces it. Something like

sg- · 2017-12-18T16:10:53Z

platform/CriticalSectionLock.h

     */
+    MBED_DEPRECATED_SINCE("mbed-os-5.8",
+            "This function is inconsistent with RAII and is being removed in the future.")


The deprecated note should have the name syntax of what replaces it. Something like

maciejbocianski · 2017-12-20T10:19:29Z

@sg- requested changes added

0xc0170 · 2017-12-22T09:17:52Z

/morph build

mbed-ci · 2017-12-22T10:22:06Z

Build : SUCCESS

Build number : 753
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5621/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2017-12-22T12:07:19Z

Exporter Build : SUCCESS

Build number : 405
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5621/

mbed-ci · 2017-12-22T15:40:32Z

Test : FAILURE

Build number : 577
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5621/577

maciejbocianski · 2017-12-22T17:04:21Z

@0xc0170
Error log

tests-netsocket-tcp_echo 
Platform: K64F - Toolchain: ARM
[1513954424.89][CONN][RXD] >>> Running case #1: 'TCP echo'... 
[1513954424.89][CONN][INF] found KV pair in stream: {{__testcase_name;TCP echo}}, queued...  
[1513954424.89][CONN][INF] found KV pair in stream: {{__testcase_start;TCP echo}}, queued... 
[1513954426.76][CONN][RXD] Thread 00000000 error -4: Parameter error 
[1513954665.00][HTST][INF] test suite run finished after 240.27 sec...

tests-netsocket-udp_dtls_handshake 
Platform: K64F - Toolchain: ARM
[1513955330.40][CONN][RXD] >>> Running case #1: 'UDP DTLS handshake'... 
[1513955330.48][CONN][INF] found KV pair in stream: {{__testcase_start;UDP DTLS handshake}}, queued... 
[1513955332.17][CONN][RXD] Thread 00000000 error -4: Parameter error 
[1513955450.29][HTST][INF] test suite run finished after 120.05 sec...

cmonr · 2017-12-28T21:04:34Z

/morph build

mbed-ci · 2017-12-28T21:29:56Z

Build : SUCCESS

Build number : 764
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5621/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2017-12-28T22:10:24Z

Exporter Build : SUCCESS

Build number : 419
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5621/

mbed-ci · 2017-12-28T22:48:02Z

Test : SUCCESS

Build number : 594
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5621/594

0xc0170 added the needs: review label Nov 30, 2017

maciejbocianski force-pushed the critical_section_imp2 branch from 3fe66fe to fa1a7ef Compare December 1, 2017 07:42

maciejbocianski mentioned this pull request Dec 4, 2017

CriticalSectionLock class improvement #5420

Closed

0xc0170 requested review from c1728p9 and deepikabhavnani December 4, 2017 12:21

maciejbocianski force-pushed the critical_section_imp2 branch from fa1a7ef to 18ee38f Compare December 4, 2017 14:40

deepikabhavnani approved these changes Dec 4, 2017

View reviewed changes

bulislaw approved these changes Dec 5, 2017

View reviewed changes

c1728p9 requested a review from sg- December 11, 2017 17:46

c1728p9 reviewed Dec 11, 2017

View reviewed changes

c1728p9 approved these changes Dec 11, 2017

View reviewed changes

sg- suggested changes Dec 16, 2017

View reviewed changes

maciejbocianski force-pushed the critical_section_imp2 branch from 18ee38f to b38f3f1 Compare December 18, 2017 07:23

0xc0170 added needs: work and removed needs: review labels Dec 18, 2017

maciejbocianski force-pushed the critical_section_imp2 branch 2 times, most recently from 98db4e5 to 7c7a046 Compare December 18, 2017 09:53

0xc0170 added needs: review and removed needs: work labels Dec 18, 2017

maciejbocianski mentioned this pull request Dec 18, 2017

Test set for critical section #5201

Merged

sg- suggested changes Dec 18, 2017

View reviewed changes

maciejbocianski force-pushed the critical_section_imp2 branch from 7c7a046 to 8091d49 Compare December 20, 2017 10:18

sg- approved these changes Dec 20, 2017

View reviewed changes

maciejbocianski force-pushed the critical_section_imp2 branch 2 times, most recently from 8aecfbd to a9a20e8 Compare December 21, 2017 13:37

0xc0170 approved these changes Dec 22, 2017

View reviewed changes

0xc0170 added release-version: 5.8.0-rc1 needs: CI and removed needs: review labels Dec 22, 2017

platform: CriticalSectionLock class improvement

ad67904

maciejbocianski force-pushed the critical_section_imp2 branch from a9a20e8 to ad67904 Compare December 23, 2017 08:19

cmonr added needs: review needs: CI and removed needs: CI needs: review labels Dec 28, 2017

cmonr added ready for merge and removed needs: CI labels Dec 28, 2017

adbridge merged commit eda0acc into ARMmbed:master Dec 29, 2017

adbridge removed the ready for merge label Dec 29, 2017

deepikabhavnani mentioned this pull request Jan 2, 2018

CriticalSection Class Update (MERGE version - mbed-os-5.8) ARMmbed/mbed-os-5-docs#370

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CriticalSectionLock class improvement #5621

CriticalSectionLock class improvement #5621

maciejbocianski commented Nov 30, 2017

0xc0170 commented Dec 1, 2017

maciejbocianski commented Dec 4, 2017

0xc0170 commented Dec 4, 2017

deepikabhavnani left a comment

c1728p9 Dec 11, 2017

maciejbocianski Dec 12, 2017

c1728p9 left a comment

sg- left a comment

maciejbocianski commented Dec 18, 2017

sg- Dec 18, 2017

sg- Dec 18, 2017

sg- Dec 18, 2017

sg- Dec 18, 2017

maciejbocianski commented Dec 20, 2017

0xc0170 commented Dec 22, 2017

mbed-ci commented Dec 22, 2017

mbed-ci commented Dec 22, 2017

mbed-ci commented Dec 22, 2017

maciejbocianski commented Dec 22, 2017 •

edited

cmonr commented Dec 28, 2017

mbed-ci commented Dec 28, 2017

mbed-ci commented Dec 28, 2017

mbed-ci commented Dec 28, 2017

CriticalSectionLock class improvement #5621

CriticalSectionLock class improvement #5621

Conversation

maciejbocianski commented Nov 30, 2017

Description

Status

Migrations

Related PRs

0xc0170 commented Dec 1, 2017

maciejbocianski commented Dec 4, 2017

0xc0170 commented Dec 4, 2017

deepikabhavnani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c1728p9 left a comment

Choose a reason for hiding this comment

sg- left a comment

Choose a reason for hiding this comment

maciejbocianski commented Dec 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maciejbocianski commented Dec 20, 2017

0xc0170 commented Dec 22, 2017

mbed-ci commented Dec 22, 2017

Triggering tests

mbed-ci commented Dec 22, 2017

mbed-ci commented Dec 22, 2017

maciejbocianski commented Dec 22, 2017 • edited

cmonr commented Dec 28, 2017

mbed-ci commented Dec 28, 2017

Triggering tests

mbed-ci commented Dec 28, 2017

mbed-ci commented Dec 28, 2017

maciejbocianski commented Dec 22, 2017 •

edited