Opened 2 months ago

Closed 2 months ago

Last modified 6 weeks ago

#2189 closed defect (fixed)

PJSUA2: thread safety issue in list of objects

Reported by: nanang Owned by: nanang
Priority: normal Milestone: release-2.9
Component: pjsua2 Version: trunk
Keywords: Cc:
Backport to 1.x milestone: Backported: no

Description

Currently PJSUA2 internally maintains some lists of objects, e.g: AudioMedia, Buddy, AudioDevInfo, VideoDevInfo, CodecInfo. Unfortunately some APIs for querying and modifying those lists are not really thread safe. For example Endpoint::mediaEnumPorts() will simply return the internal list, and Endpoint::mediaAdd(), Endpoint::mediaRemove(), Endpoint::mediaExists() may access the internal list without mutex protection. Another example is enumeration function such as Aud/VidDevManager::enumDev(), CodecInfoVector &codecEnum() that returns internal list which always be updated at each call of the function, so if two threads call the same function at the same time, one of the thread will get an invalid list. Thank you Niclas Larsson for the report.

Since those PJSUA2 objects are actually just kind of thin wrapper of PJSUA objects, e.g: PJSUA2 AudioMedia is wrapping PJSUA conference bridge port ID, those object should be easily generated from PJSUA. So instead of maintaining list of objects internally, PJSUA2 can simply generate the requested objects on the fly. For example when enumeration function is called, the function will generate the PJSUA2 objects, generate a new list containing those object, and return the list. This way PJSUA2 can also avoid adding another layer of synchronization as PJSUA already provides the thread safety.

This ticket will deprecate these APIs:

  • AudioMedia related: AudioMediaVector, Endpoint::mediaEnumPorts(), Endpoint::mediaAdd(), Endpoint::mediaRemove(), Endpoint::mediaExists(), Endpoint::typecastFromMedia(), AudioMediaPlayer::typecastFromAudioMedia(), AudioMediaRecorder::typecastFromAudioMedia(), Endpoint::registerMediaPort()
  • Buddy related: BuddyVector, Account::enumBuddies(), Account::findBuddy()
  • Codec enumeration: CodecInfoVector, Endpoint::codecEnum(), Endpoint::videoCodecEnum()
  • Audio/video device info: AudioDevInfoVector, VideoDevInfoVector, AudDevManager::enumDev(), VidDevManager::enumDev()


This ticket will introduce new APIs:

  • AudioMedia related: AudioMediaVector2, Endpoint::mediaEnumPorts2(), Endpoint::registerMediaPort2()
  • Buddy related: BuddyVector2, Account::enumBuddies2(), Account::findBuddy2()
  • Codec enumeration: CodecInfoVector2, Endpoint::codecEnum2(), Endpoint::videoCodecEnum2()
  • Audio/video device info: AudioDevInfoVector2, VideoDevInfoVector2, AudDevManager::enumDev2(), VidDevManager::enumDev2()

Change History (2)

comment:1 Changed 2 months ago by nanang

  • Owner set to nanang
  • Resolution set to fixed
  • Status changed from new to closed

In 5969:

Close #2189: fixed PJSUA2 thread safety issue in list of objects manipulation.

comment:2 Changed 6 weeks ago by ming

In 5986:

Re #2189: Fixing various bugs:

  • assertion: !Endpoint::instance().mediaExists(*this) in Media::registerMediaPort() when using AudioMedia?

Sound device is already registered in the conference bridge, while AudioMediaPlayer/Recorder? creation function, i.e. pjsua_player_create(), pjsua_playlist_create(), and pjsua_recorder_create() already call pjmedia_conf_add_port(), so mediaExists() will always return TRUE.

  • Endpoint.mediaAdd() should check in its own internal list if the media exists, rather than querying pjsua.
  • Calling Endpoint::libDestroy() first, then deleting Endpoint will cause crash, since the mutex to remove the medias has been deleted in libDestroy().

The introduction of the mutex is in r5964.

  • DevAudioMedia? never removes itself from mediaList, potentially causing infinite loop/crash.
Note: See TracTickets for help on using tickets.