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.

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

Fix nRF52 race condition / hard lock #857

Merged
merged 3 commits into from Aug 13, 2019

Conversation

@henrygab
Copy link
Contributor

commented Aug 6, 2019

Fixes #840.

A textbook case of where `volatile` is necessary

The variable s_SequenceBufferInUse is modified in an ISR.
However, the variable was not marked volatile:

virtual void showPixels(PixelController<_RGB_ORDER> & pixels) {
    while (s_SequenceBufferInUse != 0);

Because it wasn't marked volatile, the compiler was allowed to use the first value it read. Using compiler explorer set to ARM gcc 7.2.1 (none) with optimizations (e.g., -O5), you would get something similar to:

.L10
        .word .LANCHOR0 // address of .LC0
        .word .LC0             // s_SequenceBufferInUse
showPixels():
        ldr     r3, .L10
        ldr     r3, [r3]
        cmp     r3, #0
        beq     .L6
.L7:
        b       .L7 // Note this doesn't re-read the value.....
.L6:
        // remainder of function

@henrygab henrygab changed the title Fix race condition / add textbook use of 'volatile' Fix nRF52 race condition Aug 6, 2019

@henrygab henrygab changed the title Fix nRF52 race condition Fix nRF52 race condition / hard lock Aug 6, 2019

@henrygab

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

FYI -- TravisCI build succeeded.
See https://travis-ci.org/henrygab/FastLED/builds/568439071

@focalintent focalintent merged commit 535188d into FastLED:master Aug 13, 2019

@henrygab henrygab deleted the henrygab:MinFix840 branch Aug 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.