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

lib/platform/linux/platform.c: polling thread accesses indication queue variables without mutex held #26

Open
jkivilin opened this issue Feb 7, 2020 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@jkivilin
Copy link

jkivilin commented Feb 7, 2020

While doing upgrade from earlier WPC API library to c-mesh-api/master, we noticed this issue during code review.

In lib/platform/linux/platform.c, dispatch queue accesses indication queue with m_queue_mutex held. However, polling thread is accessing indication queue variables without taking mutex, for example at
https://github.com/wirepas/c-mesh-api/blob/master/lib/platform/linux/platform.c#L177

@jkivilin jkivilin added the bug Something isn't working label Feb 7, 2020
@GwendalRaoul
Copy link
Contributor

GwendalRaoul commented Aug 28, 2020

Sorry for late reply.

I don't think there is a real issue here.
The only two parts where the queue is accessed is dispatch_indication for read access (https://github.com/wirepas/c-mesh-api/blob/master/lib/platform/linux/platform.c#L86) and onIndicationReceivedLocked for write access (https://github.com/wirepas/c-mesh-api/blob/master/lib/platform/linux/platform.c#L125)

But I agree that in poll_for_indication (https://github.com/wirepas/c-mesh-api/blob/master/lib/platform/linux/platform.c#L169) it would be cleaner to held the mutex also when computing the queue size. But the size is more informative here.

Only risk is to underestimate the free space and it is not an issue.

GwendalRaoul pushed a commit that referenced this issue Aug 28, 2020
It was not clear why the indication queue is not locked when checking
free space. #26
GwendalRaoul pushed a commit that referenced this issue Sep 14, 2020
It was not clear why the indication queue is not locked when checking
free space. #26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants