Free the arbitrated client token from client manager #444
Conversation
MichalPrincNXP
force-pushed
the
token_memLeak
branch
from
9a1572c to
86401f6
Compare
MichalPrincNXP
force-pushed
the
token_memLeak
branch
from
86401f6 to
672aa84
Compare
|
Thank you for the effort. |
| PendingClientInfo *node; | ||
|
|
||
| erpc_assert((token != 0) && ("invalid client token" != NULL)); | ||
| erpc_assert((info->m_sem.getCount() == 0) && ("Semaphore should be clean" != NULL)); |
There was a problem hiding this comment.
This assert will trigger if we follow the non status ok path here:https://github.com/EmbeddedRPC/erpc/pull/444/files#diff-74a78bba1605a2e491d18277550d2c7db7943691c691d14d7a732558f9a0c681L83. Since then nothing has called received. But something may have set a result.
There was a problem hiding this comment.
Sorry @elupus , I don't understand you, can you please explain the flow deeply please?
There was a problem hiding this comment.
If the arbitrator->send call fails for whatever reason. The buffer will be signalled as failed. However the buffer has already been handed of to handed of to another thread (the receiving thread). This other thread can then just have received data that complete this request or it can just have received some aborting read condition.
The race condition ls very tight with master of erpc (but very much there), if you fix some other race conditions like making sure errors are propagated from the arbitrator, the race condition window grows.
In general, if we handed the buffer to another thread we shall always remove it as well. How it was finalized is nothing that make sense to assert on (warning could be fine)
There was a problem hiding this comment.
Sorry @elupus , I still don't understand.
On which of the two asserts you talking, the token or semaphore? and if send failed, why other thread has the buffer (and what is your system, the sender and reciever are on same CPU?)
Is this issue that happened to you or theoretical issue?
Can you please explain step by step, with permalink to the relevant lines?
There was a problem hiding this comment.
This assert will trigger if we follow the non status ok path here:https://github.com/EmbeddedRPC/erpc/pull/444/files#diff-74a78bba1605a2e491d18277550d2c7db7943691c691d14d7a732558f9a0c681L83. Since then nothing has called received. But something may have set a result.
I have the same problem assert,How should we solve it?@elupus
There was a problem hiding this comment.
I will check how we handle this internally. Since #469 stalled which is semi related i got sidetracked.
4 participants
Pull request
Choose Correct
Describe the pull request
Closes #443 , Make the token free be from the client management and it will be not matter what is the status
To Reproduce
Expected behavior
Screenshots
Desktop (please complete the following information):
Steps you didn't forgot to do
Additional context