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

ASR platform manager synchronization seems to be completely broken #26637

Closed
bzbarsky-apple opened this issue May 17, 2023 · 5 comments · Fixed by #26645
Closed

ASR platform manager synchronization seems to be completely broken #26637

bzbarsky-apple opened this issue May 17, 2023 · 5 comments · Fixed by #26645

Comments

@bzbarsky-apple
Copy link
Contributor

See #26361 (review)

@tx2rx @andy31415

@bzbarsky-apple
Copy link
Contributor Author

Also, where is _TryLockChipStack implemented for ASR? I don't see it.

@bzbarsky-apple
Copy link
Contributor Author

Note that I only reviewed PlatformManager and TimeSupport from #26361 and both were broken. I have extremely low confidence in any of the rest of it...

@tx2rx
Copy link
Contributor

tx2rx commented May 17, 2023

Also, where is _TryLockChipStack implemented for ASR? I don't see it.

Because in src/ , that is not used before. Just in case ,we will add it soon later

@tx2rx
Copy link
Contributor

tx2rx commented May 18, 2023

In addition, src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.ipp may cause the same problem:

    BaseType_t eventReceived = pdFALSE;
    {
        // Unlock the CHIP stack, allowing other threads to enter CHIP while
        // the event loop thread is sleeping.
        StackUnlock unlock;
        eventReceived = xQueueReceive(mChipEventQueue, &event, waitTime);
    }

StackLock lock; // should it open lock again? @bzbarsky-apple

@bzbarsky-apple
Copy link
Contributor Author

bzbarsky-apple commented May 18, 2023

// should it open lock again?

It does, in the destructor of the StackLock. And temporarily while the StackUnlock is in scope, because the thread can end up sleeping under that xQueueReceive call, and sleeping should not be done with a lock held.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants