Skip to content

Commit

Permalink
Fix memory leak in SDK layer (#234)
Browse files Browse the repository at this point in the history
  • Loading branch information
@hokyungh
hokyungh authored Feb 12, 2021
1 parent 77e4b7f commit 485f4d0
Show file tree
Hide file tree
Showing 15 changed files with 103 additions and 27 deletions.
8 changes: 4 additions & 4 deletions AmazonChimeSDK/AmazonChimeSDK/audiovideo/DefaultAudioVideoController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,20 @@ import Foundation
private let audioClientObserver: AudioClientObserver
private let clientMetricsCollector: ClientMetricsCollector
private var videoClientController: VideoClientController
private let videoTileController: VideoTileController

public init(audioClientController: AudioClientController,
audioClientObserver: AudioClientObserver,
clientMetricsCollector: ClientMetricsCollector,
videoClientController: VideoClientController,
videoTileController: VideoTileController,
configuration: MeetingSessionConfiguration,
logger: Logger) {
self.audioClientController = audioClientController
self.audioClientObserver = audioClientObserver
self.clientMetricsCollector = clientMetricsCollector
self.videoClientController = videoClientController
self.videoTileController = videoTileController
self.configuration = configuration
self.logger = logger
}
Expand All @@ -39,16 +42,13 @@ import Foundation
}

public func start(callKitEnabled: Bool) throws {
if let audioClientObserver = audioClientObserver as? DefaultAudioClientObserver {
DefaultAudioClient.shared(logger: logger).delegate = audioClientObserver
}

try audioClientController.start(audioFallbackUrl: configuration.urls.audioFallbackUrl,
audioHostUrl: configuration.urls.audioHostUrl,
meetingId: configuration.meetingId,
attendeeId: configuration.credentials.attendeeId,
joinToken: configuration.credentials.joinToken,
callKitEnabled: callKitEnabled)
videoClientController.subscribeToVideoTileControllerObservers(observer: videoTileController)
videoClientController.start()
}

Expand Down
1 change: 1 addition & 0 deletions AmazonChimeSDK/AmazonChimeSDK/audiovideo/DefaultAudioVideoFacade.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import Foundation

public func start(callKitEnabled: Bool = false) throws {
try audioVideoController.start(callKitEnabled: callKitEnabled)

trace(name: "start(callKitEnabled: \(callKitEnabled))")
}

Expand Down
9 changes: 0 additions & 9 deletions .../AmazonChimeSDK/audiovideo/audio/activespeakerdetector/DefaultActiveSpeakerDetector.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,15 @@ typealias DetectorCallback = (_ attendeeIds: [AttendeeInfo]) -> Void
private let scoresTimers = ConcurrentDictionary<String, Scheduler>()
private var hasBandwidthPriority = false
private let mostRecentUpdateTimestamp = ConcurrentDictionary<AttendeeInfo, TimeInterval>()
private let audioClientObserver: AudioClientObserver
private let selfAttendeeId: String
private let policiesAndCallbacks = ConcurrentDictionary<String, (ActiveSpeakerPolicy, DetectorCallback)>()
private var timer = IntervalScheduler(intervalMs: activityWaitIntervalMs, callback: {})

public init(
audioClientObserver: AudioClientObserver,
selfAttendeeId: String
) {
self.audioClientObserver = audioClientObserver
self.selfAttendeeId = selfAttendeeId
super.init()
audioClientObserver.subscribeToRealTimeEvents(observer: self)

timer = IntervalScheduler(
intervalMs: DefaultActiveSpeakerDetector.activityUpdateIntervalMs,
Expand All @@ -60,11 +56,6 @@ typealias DetectorCallback = (_ attendeeIds: [AttendeeInfo]) -> Void
timer.start()
}

deinit {
audioClientObserver.unsubscribeFromRealTimeEvents(observer: self)
timer.stop()
}

private func needUpdateActiveSpeakers(attendeeInfo: AttendeeInfo) -> Bool {
if activeSpeakers.isEmpty {
return true
Expand Down
25 changes: 24 additions & 1 deletion AmazonChimeSDK/AmazonChimeSDK/internal/audio/DefaultAudioClientController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,25 @@ class DefaultAudioClientController: NSObject {
private let defaultPresenter = true
private let eventAnalyticsController: EventAnalyticsController
private let meetingStatsCollector: MeetingStatsCollector
private let activeSpeakerDetector: ActiveSpeakerDetectorFacade
private let logger: Logger

init(audioClient: AudioClientProtocol,
audioClientObserver: AudioClientObserver,
audioSession: AudioSession,
audioClientLock: AudioLock,
eventAnalyticsController: EventAnalyticsController,
meetingStatsCollector: MeetingStatsCollector) {
meetingStatsCollector: MeetingStatsCollector,
activeSpeakerDetector: ActiveSpeakerDetectorFacade,
logger: Logger) {
self.audioClient = audioClient
self.audioClientObserver = audioClientObserver
self.audioSession = audioSession
audioLock = audioClientLock
self.eventAnalyticsController = eventAnalyticsController
self.meetingStatsCollector = meetingStatsCollector
self.activeSpeakerDetector = activeSpeakerDetector
self.logger = logger
super.init()
}
}
Expand Down Expand Up @@ -66,6 +72,14 @@ extension DefaultAudioClientController: AudioClientController {
throw MediaError.illegalState
}

if let defaultAudioClientObserver = audioClientObserver as? DefaultAudioClientObserver {
DefaultAudioClient.shared(logger: logger).delegate = defaultAudioClientObserver
}

if let observer = activeSpeakerDetector as? RealtimeObserver {
audioClientObserver.subscribeToRealTimeEvents(observer: observer)
}

let url = audioHostUrl.components(separatedBy: ":")
let host = url[0]
let port = url.count >= 2 ? (url[1] as NSString).integerValue - audioPortOffset : defaultPort
Expand All @@ -91,6 +105,7 @@ extension DefaultAudioClientController: AudioClientController {
Self.state = .started
} else {
eventAnalyticsController.publishEvent(name: .meetingStartFailed)
cleanup()
throw MediaError.audioFailedToStart
}
}
Expand All @@ -114,6 +129,7 @@ extension DefaultAudioClientController: AudioClientController {
sessionStatus: MeetingSessionStatus(statusCode: meetingSessionStatusCode)
)
}
self.cleanup()
}
}

Expand All @@ -138,4 +154,11 @@ extension DefaultAudioClientController: AudioClientController {
return false
}
}

private func cleanup() {
if let observer = self.activeSpeakerDetector as? RealtimeObserver {
self.audioClientObserver.unsubscribeFromRealTimeEvents(observer: observer)
}
DefaultAudioClient.shared(logger: self.logger).delegate = nil
}
}
8 changes: 7 additions & 1 deletion ...meSDK/AmazonChimeSDK/internal/contentshare/DefaultContentShareVideoClientController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import Foundation
self.clientMetricsCollector = clientMetricsCollector
self.videoClient = videoClient
super.init()
videoClient.delegate = self
videoClient.setReceiving(false)
}

Expand Down Expand Up @@ -64,6 +63,7 @@ import Foundation
}

private func startVideoClient() {
videoClient.delegate = self
videoClient.start(configuration.meetingId,
token: configuration.credentials.joinToken,
sending: false,
Expand Down Expand Up @@ -123,6 +123,7 @@ extension DefaultContentShareVideoClientController: VideoClientDelegate {
observer.contentShareDidStop(status: ContentShareStatus(statusCode: .videoServiceFailed))
}
isSharing = false
cleanUp()
}

public func videoClientDidStop(_ client: VideoClient?) {
Expand All @@ -132,6 +133,7 @@ extension DefaultContentShareVideoClientController: VideoClientDelegate {
observer.contentShareDidStop(status: ContentShareStatus(statusCode: .ok))
}
isSharing = false
cleanUp()
}

public func videoClientMetricsReceived(_ metrics: [AnyHashable: Any]?) {
Expand All @@ -142,4 +144,8 @@ extension DefaultContentShareVideoClientController: VideoClientDelegate {
private func resetContentShareVideoClientMetrics() {
clientMetricsCollector.processContentShareVideoClientMetrics(metrics: [:])
}

private func cleanUp() {
videoClient.delegate = nil
}
}
6 changes: 6 additions & 0 deletions AmazonChimeSDK/AmazonChimeSDK/internal/utils/ConcurrentMutableSet.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ import Foundation
defer { lock.unlock() }
set.remove(object)
}

func removeAll() {
lock.lock()
defer { lock.unlock() }
set.removeAllObjects()
}

func contains(_ object: Any) -> Bool {
return set.contains(object)
Expand Down
2 changes: 2 additions & 0 deletions AmazonChimeSDK/AmazonChimeSDK/internal/video/DefaultVideoClientController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ class DefaultVideoClientController: NSObject {

private func destroyVideoClient() {
logger.info(msg: "VideoClient is being destroyed")
videoTileControllerObservers.removeAll()
videoClient?.delegate = nil
videoClient = nil
videoClientState = .uninitialized
}
Expand Down
11 changes: 6 additions & 5 deletions AmazonChimeSDK/AmazonChimeSDK/session/DefaultMeetingSession.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,16 @@ import Foundation
logger: logger,
eventAnalyticsController: self.eventAnalyticsController,
meetingStatsCollector: meetingStatsCollector)
let activeSpeakerDetector =
DefaultActiveSpeakerDetector(selfAttendeeId: configuration.credentials.attendeeId)
let audioClientController = DefaultAudioClientController(audioClient: audioClient,
audioClientObserver: audioClientObserver,
audioSession: audioSession,
audioClientLock: audioClientLock,
eventAnalyticsController: self.eventAnalyticsController,
meetingStatsCollector: meetingStatsCollector)
meetingStatsCollector: meetingStatsCollector,
activeSpeakerDetector: activeSpeakerDetector,
logger: logger)
let videoClientController = DefaultVideoClientController(videoClient: videoClient,
clientMetricsCollector: clientMetricsCollector,
configuration: configuration,
Expand All @@ -54,13 +58,9 @@ import Foundation
DefaultVideoTileController(videoClientController: videoClientController,
logger: logger,
meetingStatsCollector: meetingStatsCollector)
videoClientController.subscribeToVideoTileControllerObservers(observer: videoTileController)
let realtimeController = DefaultRealtimeController(audioClientController: audioClientController,
audioClientObserver: audioClientObserver,
videoClientController: videoClientController)
let activeSpeakerDetector =
DefaultActiveSpeakerDetector(audioClientObserver: audioClientObserver,
selfAttendeeId: configuration.credentials.attendeeId)

let contentModality = String(DefaultModality.separator) + String(describing: ModalityType.content)
let contentShareCredentials = MeetingSessionCredentials(
Expand All @@ -85,6 +85,7 @@ import Foundation
audioClientObserver: audioClientObserver,
clientMetricsCollector: clientMetricsCollector,
videoClientController: videoClientController,
videoTileController: videoTileController,
configuration: configuration,
logger: logger),
realtimeController: realtimeController,
Expand Down
3 changes: 3 additions & 0 deletions AmazonChimeSDK/AmazonChimeSDKTests/audiovideo/DefaultAudioVideoControllerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class DefaultAudioVideoControllerTests: CommonTestCase {
var audioClientObserverMock: AudioClientObserverMock!
var clientMetricsCollectorMock: ClientMetricsCollectorMock!
var videoClientControllerMock: VideoClientControllerMock!
var videoTileControllerMock: VideoTileControllerMock!
var defaultAudioClientMock: DefaultAudioClientMock!
var defaultAudioVideoController: DefaultAudioVideoController!

Expand All @@ -26,11 +27,13 @@ class DefaultAudioVideoControllerTests: CommonTestCase {
audioClientObserverMock = mock(AudioClientObserver.self)
clientMetricsCollectorMock = mock(ClientMetricsCollector.self)
videoClientControllerMock = mock(VideoClientController.self)
videoTileControllerMock = mock(VideoTileController.self)

defaultAudioVideoController = DefaultAudioVideoController(audioClientController: audioClientControllerMock,
audioClientObserver: audioClientObserverMock,
clientMetricsCollector: clientMetricsCollectorMock,
videoClientController: videoClientControllerMock,
videoTileController: videoTileControllerMock,
configuration: meetingSessionConfigurationMock,
logger: loggerMock)
}
Expand Down
8 changes: 4 additions & 4 deletions AmazonChimeSDK/AmazonChimeSDKTests/audiovideo/audio/ActiveSpeakerDetectorTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ class ActiveSpeakerDetectorTests: XCTestCase, ActiveSpeakerPolicy, ActiveSpeaker
var attendeesReceived: [AttendeeInfo] = [AttendeeInfo(attendeeId: "", externalUserId: "")]
var scoreChangeAttendees: [AttendeeInfo: Double] = [AttendeeInfo(attendeeId: "", externalUserId: ""): 0.0]
var scoreIndex = 0
var activeSpeakerDetector = DefaultActiveSpeakerDetector(
audioClientObserver: MockAudioClientObserver(), selfAttendeeId: ""
)
var activeSpeakerDetector = DefaultActiveSpeakerDetector(selfAttendeeId: "")

private let emptyAttendeesReceivedExpectation = XCTestExpectation(
description: "Is fulfilled when received no attendees in callback")
private let calculateScoreExpectation = XCTestExpectation(
Expand Down Expand Up @@ -60,7 +59,8 @@ class ActiveSpeakerDetectorTests: XCTestCase, ActiveSpeakerPolicy, ActiveSpeaker
prioritizeBandwidthExpectation.assertForOverFulfill = true
subscribeToRealTimeEventsExpectation.assertForOverFulfill = true
unSubscribeFromRealTimeEventsExpectation.assertForOverFulfill = true
activeSpeakerDetector = DefaultActiveSpeakerDetector(audioClientObserver: self, selfAttendeeId: "attendee0")
activeSpeakerDetector = DefaultActiveSpeakerDetector(selfAttendeeId: "attendee0")
self.subscribeToRealTimeEvents(observer: activeSpeakerDetector)
activeSpeakerDetector.addActiveSpeakerObserver(policy: self, observer: self)
activeSpeakerDetector.attendeesDidJoin(attendeeInfo: attendees)
}
Expand Down
7 changes: 6 additions & 1 deletion AmazonChimeSDK/AmazonChimeSDKTests/internal/audio/DefaultAudioClientControllerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class DefaultAudioClientControllerTests: CommonTestCase {
var audioClientObserverMock: AudioClientObserverMock!
var audioSessionMock: AudioSessionMock!
var audioLockMock: AudioLockMock!
var activeSpeakerMock: ActiveSpeakerDetectorFacadeMock!

var eventAnalyticsControllerMock: EventAnalyticsControllerMock!
var meetingStatsCollectorMock: MeetingStatsCollectorMock!

Expand All @@ -32,6 +34,7 @@ class DefaultAudioClientControllerTests: CommonTestCase {
audioLockMock = mock(AudioLock.self)
eventAnalyticsControllerMock = mock(EventAnalyticsController.self)
meetingStatsCollectorMock = mock(MeetingStatsCollector.self)
activeSpeakerMock = mock(ActiveSpeakerDetectorFacade.self)

given(meetingStatsCollectorMock.getMeetingStats()).will { return [AnyHashable: Any]() }

Expand All @@ -40,7 +43,9 @@ class DefaultAudioClientControllerTests: CommonTestCase {
audioSession: audioSessionMock,
audioClientLock: audioLockMock,
eventAnalyticsController: eventAnalyticsControllerMock,
meetingStatsCollector: meetingStatsCollectorMock)
meetingStatsCollector: meetingStatsCollectorMock,
activeSpeakerDetector: activeSpeakerMock,
logger: loggerMock)
}

func testSetMute_stateInitialized() {
Expand Down
1 change: 1 addition & 0 deletions AmazonChimeSDKDemo/AmazonChimeSDKDemo/InAppScreenCaptureModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ extension InAppScreenCaptureModel: CaptureSourceObserver {
func captureDidStop() {
logger.info(msg: "InAppScreenCaptureSource did stop")
contentShareController.stopContentShare()
inAppScreenCaptureSource?.removeCaptureSourceObserver(observer: self)
}

func captureDidFail(error: CaptureSourceError) {
Expand Down
4 changes: 2 additions & 2 deletions AmazonChimeSDKDemo/AmazonChimeSDKDemo/MeetingModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ class MeetingModel: NSObject {

private var isEnded = false {
didSet {
// This will unbind current tiles.
videoModel.isEnded = true
currentMeetingSession.audioVideo.stop()
screenShareModel.stopLocalSharing()
videoModel.customSource.stop()
videoModel.customSource.torchEnabled = false
isEndedHandler?()
}
}
Expand Down
25 changes: 25 additions & 0 deletions AmazonChimeSDKDemo/AmazonChimeSDKDemo/VideoModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,31 @@ class VideoModel: NSObject {
}
}

var isEnded = false {
didSet(isEnded) {
if isEnded {
for tile in remoteVideoTileStates {
audioVideoFacade.unbindVideoView(tileId: tile.0)
}
if isLocalVideoActive, let selfTile = selfVideoTileState {
audioVideoFacade.unbindVideoView(tileId: selfTile.tileId)
}

if isUsingExternalVideoSource {
self.customSource.removeVideoSink(sink: self.coreImageVideoProcessor)
if isUsingMetalVideoProcessor, let metalProcessor = self.metalVideoProcessor {
self.customSource.removeVideoSink(sink: metalProcessor)
}
}

isLocalVideoActive = false

audioVideoFacade.stopRemoteVideo()
customSource.torchEnabled = false
}
}
}

var isFrontCameraActive: Bool {
// See comments above isUsingExternalVideoSource
if let internalCamera = audioVideoFacade.getActiveCamera() {
Expand Down
Loading

0 comments on commit 485f4d0

Please sign in to comment.