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

udp: add a lock for the helpers list #732

Merged
merged 5 commits into from
Mar 28, 2023
Merged

Conversation

cspiel1
Copy link
Collaborator

@cspiel1 cspiel1 commented Mar 7, 2023

relates to RX-thread: baresip/baresip#2445

The helpers list may be changed from main thread while the RX thread processes the helpers.

Edit: More concretely the bundle feature in baresip could add the bundle helper during the call, with UPDATE or re-INVITE.

@cspiel1 cspiel1 added the RX thread RX real-time thread label Mar 7, 2023
src/udp/udp.c Outdated Show resolved Hide resolved
src/udp/udp.c Outdated Show resolved Hide resolved
@cspiel1
Copy link
Collaborator Author

cspiel1 commented Mar 10, 2023

list_unlink() should also be locked.

@sreimers sreimers added this to the v2.14.0 milestone Mar 12, 2023
@cspiel1
Copy link
Collaborator Author

cspiel1 commented Mar 15, 2023

There is a dead lock currently in dtls_estab_handler() with selftest.

@cspiel1 cspiel1 marked this pull request as draft March 15, 2023 17:18
@cspiel1
Copy link
Collaborator Author

cspiel1 commented Mar 16, 2023

Solution: Maybe two different locks

  • a lock for the helpers list which should not be locked while the helper callback is called
  • a second lock that protects the helper itself to be deleted while the helper callback is called

Edit:
The implementation of the helper has to take care that it is not deleted while it is in the callback.

This protects changes of prev and next pointer of the helpers list.
@cspiel1
Copy link
Collaborator Author

cspiel1 commented Mar 16, 2023

The new solution protects only changes of prev and next pointer. Should we use atomic?

Protection against helper deletion vs. processing concurrency has to be done in the upper layer.

@cspiel1 cspiel1 marked this pull request as ready for review March 17, 2023 08:34
@sreimers sreimers merged commit f71ac2f into baresip:main Mar 28, 2023
@cspiel1 cspiel1 deleted the udp_sock_lock branch March 29, 2023 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RX thread RX real-time thread
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants