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

Fixed Buddy to use correct account during subscription #4227

Merged
merged 4 commits into from
Dec 23, 2024
Merged

Conversation

sauwming
Copy link
Member

@sauwming sauwming commented Dec 20, 2024

To fix #4226

The issue occurs because in pjsua, buddy is not tied to any account, so it will always call pjsua_acc_find_for_outgoing() to find the appropriate account for outgoing subscription.

Also in this PR:
Fixed bug in Account::findBuddy2() and enumBuddies2() to only return buddies associated with the account.

@nanangizz
Copy link
Member

What happens to the buddy when the account is deleted?

@sauwming
Copy link
Member Author

What happens to the buddy when the account is deleted?

Good question. I just add validity check in pjsua. In pjsua2 though, if the associated Account instance has been destroyed, then it will be a problem indeed, though I suppose it will be the app developer's fault? The best we can do is add doc in Buddy::create().

@nanangizz
Copy link
Member

  • The validity check may not work properly (returning false validity) when the original & deleted account ID is being reused.
  • Alongside docs, perhaps what if account deletion also raises assertion (and return an error) for PJSUA & exception for PJSUA2 if the account still has buddies referring to it? For app using PJSUA2 this might be a surprise. For app using PJSUA this should not be a problem as linking buddy to account is new in this PR.

@sauwming
Copy link
Member Author

I tried returning error but encountered a few issues:

  • Account::shutdown() doesn't throw error.
  • Most of the time, people don't expect deletion to fail. So they don't check return status of pjsua_acc_del() and also there's no retry mechanism, so they won't expect that the account is still there until they run out of capacity.

So, I opted to remove the buddy-account association. There's still a possibility of race, but then the doc has specifically warned (in three locations now) to keep the Account alive when Buddy is alive:

* create()) will unregister and delete the buddy from PJSUA-LIB. Application
* must delete all Buddy instances belong to an account before shutting down
* the account (via Account::shutdown()).

(the other two warnings in pjsua_buddy_config and Buddy::Create() in this patch).

Also fixed bug in Account::findBuddy2() and enumBuddies2() to only return buddies associated with the account.

@sauwming sauwming merged commit e9060fb into master Dec 23, 2024
36 checks passed
@sauwming sauwming deleted the buddy-acc branch December 23, 2024 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PJSUA2 ignores the account set on Buddy when subscribing
3 participants