do not unregister EA accessory notification on transport dealloc #1315

t-yoshii · 2019-06-21T04:15:27Z

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

We verified this fix with our devboard and OEM HU.

Summary

Calling EAAccessoryManager#unregisterForLocalNotifications makes all SDLIAPTransport instances not be able to receive EA events.

Changelog

Bug Fixes

Fix the issue that SDLIAPTransport does not receive EA events after transport reconnect.

CLA

I have signed the CLA

joeljfischer · 2019-06-21T15:56:00Z

Would a better fix be to change:

self.backgroundTaskId = [[UIApplication sharedApplication] beginBackgroundTaskWithName:BackgroundTaskName expirationHandler:^{
    SDLLogD(@"Background task expired");
    [self sdl_backgroundTaskEnd];
}];

to capture self weakly and use [weakself sdl_backgroundTaskEnd];?

Would that solve the problem of delaying the deallocation of SDLIAPTransport? It seems to work in my testing.

t-yoshii · 2019-06-24T00:44:06Z

@joeljfischer ,

Thanks for your comments.

I think capturing self weakly will solve the delayed deallocation of SDLIAPTransport, however sdl_backgroundTaskEnd will not be called when the background task expires because weakself is nil at that timing.

joeljfischer · 2019-06-24T14:44:50Z

But that's fine, because it is called in dealloc:

- (void)dealloc {
    SDLLogV(@"SDLIAPTransport dealloc");
    [self disconnect];
    [self sdl_backgroundTaskEnd];
    self.controlSession = nil;
    self.dataSession = nil;
    self.delegate = nil;
    self.sessionSetupInProgress = NO;
    self.accessoryConnectDuringActiveSession = NO;
}

t-yoshii · 2019-06-25T01:02:26Z

Oh, understand. I will retest with the suggested solution. Thanks.

joeljfischer · 2019-06-26T14:51:26Z

Did the merge of #1317 solve this issue for you?

t-yoshii · 2019-07-01T06:22:59Z

Hi @joeljfischer ,

Thanks for the update. We tested with 6.3.0 and found some problems.

When the delayed sdl_connect by old SDLIAPTransport instance is called after the reconnection by another SDLIAPTransport is done, deallc of old SDLIAPTransport occurs and causes unregistration of EAAccessoryManager. This prevents the succeeding EA events detection. Please check the following logs.

In addition, when other components other than SDL uses EA Accessory, EAAccessoryManager#unregisterForLocalNotifications also affects the EA event listening of such components.
One of our SDL apps uses EA accessory other than SDL. The application sometimes stops SDLManager (this lead to call EAAccessoryManager#unregisterForLocalNotifications) in some situation, but it still needs to use EA accessory for the other purpose (with other protocol strings.)

Therefore, in any way, I think SDL proxy framework should not call EAAccessoryManager#unregisterForLocalNotifications.

// Connect USB, iOS detects EAAccessoryArrived (USB) and EAAccessoryLeft (BT)
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(CoreAccessories)[691] <Notice>: accessoryd received EAAccessoryArrived <private>
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(CoreAccessories)[691] <Notice>: accessoryd received EAAccessoryLeft <private>, UUID <private>
 :
// EAAccessoryLeft event is processed
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:645 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPTransport:135 - Accessory with serial number: 1234567890, and connectionID: 37965985 disconnecting.
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:645 \M-b\M^Z\M-* (SDL)Transport:SDLIAPTransport:147 - Accessory (SABRE, 1234567890) disconnected during a data session
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:645 \M-b\M^Z\M-* (SDL)Transport:SDLIAPTransport:82 - SDLIAPTransport stopped listening for events
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:645 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPDataSession:77 - Destroying the data session
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:668 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPDataSession:339 - Closing the accessory event loop on thread: com.smartdevicelink.iostream
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:668 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPDataSession:354 - Closing EASession for accessory connection id: 37965985, name: SABRE
 :
//
// EAAccessoryArrived event is processed
// Timer for `sdl_connect` is set.
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:678 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPTransport:102 - Accessory Connected (<EAAccessory: 0x2834803d0> { 
  connected:YES 
  connectionID:37965986 
  name: SABRE 
  manufacturer: Xevo 
  modelNumber: ToyotaCTS 
  serialNumber: 1234567890 
  ppid: (null) 
  firmwareRevisionActive: \134^A. 
  firmwareRevisionPending: (null) 
  hardwareRevision: \134^A. 
  dockType:  
  certSerial: 15 bytes 
  certData: 908 bytes 
  protocols: (
    "com.smartdevicelink.multisession",
    "com.xevokk-jtc.capp"
) 
  delegate: (null) 
}), Opening in 8.655s
 :
 :
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:684 \M-p\M^_\M^T\M-5 (SDL)Lifecycle:SDLLifecycleManager:720 - Transport Disconnected
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:684 \M-b\M^Z\M-* (SDL)Utilities:SDLStateMachine:84 - State machine for class SDLLifecycleManager will transition from state Ready to state Reconnecting
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:684 \M-b\M^Z\M-* (SDL)Lifecycle:SDLLifecycleManager:248 - Stopping manager, will restart
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:685 \M-b\M^Z\M-* (SDL)Transport:SDLIAPTransport:82 - SDLIAPTransport stopped listening for events
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:685 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPDataSession:77 - Destroying the data session
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:698 \M-b\M^Z\M-* (SDL)Transport:SDLIAPDataSession:102 - Stopping data session but no thread established.
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:698 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPSession:84 - Attempting to close session with accessory: SABRE, but it is already closed
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:717 \M-b\M^Z\M-* (SDL)Proxy:SDLProxy:139 - Proxy dealloc
Jul  1 11:55:05 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:05:717 \M-b\M^Z\M-* (SDL)Utilities:SDLStateMachine:84 - State machine for class SDLFileManager will transition from state Ready to state Shutdown
Jul  1 11:55:06 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:06:450 \M-b\M^Z\M-* (SDL)Utilities:SDLStateMachine:84 - State machine for class SDLLifecycleManager will transition from state Reconnecting to state Started
 :
Jul  1 11:55:06 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:06:638 \M-p\M^_\M^T\M-5 (SDL)Lifecycle:SDLLifecycleManager:709 - Transport connected
Jul  1 11:55:06 iPhoneXS-M00326 assertiond[65] <Notice>: [CTSSimpleApp_ms:691] Deactivate assertion: <BKProcessAssertion: 0x1047240e0; "Shared Background Assertion 5 for UIEvolution.CTS-Simple-App" (finishTask:180s); id:\M-b\M^@\M-&8F7D536FC850>
Jul  1 11:55:06 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:06:638 \M-p\M^_\M^T\M-5 (SDL):SDLBackgroundTaskManager:57 - Ending background task with id: 17
Jul  1 11:55:06 iPhoneXS-M00326 assertiond[65] <Notice>: [CTSSimpleApp_ms:691] Setting jetsam priority to 3 [0x4]
Jul  1 11:55:06 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:06:638 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLSecondaryTransportManager:437 - Starting service 0xA on primary transport
Jul  1 11:55:06 iPhoneXS-M00326 assertiond[65] <Notice>: Updating PowerAssertion on CTSSimpleApp_ms:691
Jul  1 11:55:06 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:06:639 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLSecondaryTransportManager:437 - Starting service 0xB on primary transport
Jul  1 11:55:06 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:06:639 \M-b\M^Z\M-* (SDL)Utilities:SDLStateMachine:84 - State machine for class SDLSecondaryTransportManager will transition from state Started to state Configured
Jul  1 11:55:06 iPhoneXS-M00326 assertiond[65] <Notice>: [CTSSimpleApp_ms:691] Remove assertion: <BKProcessAssertion: 0x1047240e0; "Shared Background Assertion 5 for UIEvolution.CTS-Simple-App" (finishTask:180s); id:\M-b\M^@\M-&8F7D536FC850>
Jul  1 11:55:06 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:06:639 \M-b\M^Z\M-* (SDL)Utilities:SDLStateMachine:84 - State machine for class SDLLifecycleManager will transition from state Started to state Connected
 :
 :
// Timer for `sdl_connect` fired and this causes `SDLIAPTransport#dealloc` and EAAccessoryManager unregistration.
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:337 \M-b\M^Z\M-* (SDL)Transport:SDLIAPTransport:229 - Will not attempt to connect to an accessory because the data session disconnected. Waiting for lifecycle manager to create a new transport object.
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:337 \M-b\M^Z\M-* (SDL)Transport:SDLIAPTransport:517 - SDLIAPTransport dealloc
// This prevents succeeding EA event detection.
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:337 \M-b\M^Z\M-* (SDL)Transport:SDLIAPTransport:82 - SDLIAPTransport stopped listening for events
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:338 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPDataSession:77 - Destroying the data session
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:338 \M-b\M^Z\M-* (SDL)Transport:SDLIAPDataSession:102 - Stopping data session but no thread established.
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:338 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPSession:84 - Attempting to close session with accessory: SABRE, but it is already closed
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:338 \M-b\M^Z\M-* (SDL)Transport:SDLIAPDataSession:373 - SDLIAPDataSession dealloc
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:338 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPDataSession:77 - Destroying the data session
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:338 \M-b\M^Z\M-* (SDL)Transport:SDLIAPDataSession:102 - Stopping data session but no thread established.
Jul  1 11:55:14 iPhoneXS-M00326 CTSSimpleApp_ms(SmartDeviceLink)[691] <Notice>: 11:55:14:338 \M-p\M^_\M^T\M-5 (SDL)Transport:SDLIAPSession:84 - Attempting to close session with accessory: SABRE, but it is already closed

theresalech · 2019-07-08T14:03:26Z

@t-yoshii if this is not covered by any already submitted issues, please submit a new issue. We will close PR #1315, as it was confirmed Issue #1311 is resolved by PR #1317.

t-yoshii · 2019-07-09T01:56:30Z

@theresalech thanks for the update.

I recreated #1329 & #1330.

do not unregister EA accessory notification on transport dealloc

faf824d

joeljfischer added the bug A defect in the library label Jun 21, 2019

theresalech closed this Jul 8, 2019

t-yoshii mentioned this pull request Jul 9, 2019

EAAccessoryManager#unregisterForLocalNotifications should not be called in SDLIAPTransport#dealloc #1329

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

do not unregister EA accessory notification on transport dealloc #1315

do not unregister EA accessory notification on transport dealloc #1315

t-yoshii commented Jun 21, 2019

joeljfischer commented Jun 21, 2019

t-yoshii commented Jun 24, 2019

joeljfischer commented Jun 24, 2019

t-yoshii commented Jun 25, 2019

joeljfischer commented Jun 26, 2019

t-yoshii commented Jul 1, 2019

theresalech commented Jul 8, 2019

t-yoshii commented Jul 9, 2019

do not unregister EA accessory notification on transport dealloc #1315

do not unregister EA accessory notification on transport dealloc #1315

Conversation

t-yoshii commented Jun 21, 2019

Risk

Testing Plan

Summary

Changelog

Bug Fixes

CLA

joeljfischer commented Jun 21, 2019

t-yoshii commented Jun 24, 2019

joeljfischer commented Jun 24, 2019

t-yoshii commented Jun 25, 2019

joeljfischer commented Jun 26, 2019

t-yoshii commented Jul 1, 2019

theresalech commented Jul 8, 2019

t-yoshii commented Jul 9, 2019