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 crash when an iAP control session is stopped #1592

Merged
merged 2 commits into from Apr 14, 2020

Conversation

NicoleYarroch
Copy link
Contributor

Fixes #1583

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).

Unit Tests

No additional test cases were added. Current unit tests passed.

Core Tests

  1. Connected to a SYNC 3 v3.0 head unit and used code injection in the Xcode debugger to make the library create an iAP control session.
  2. Repeatedly connected and pulled the USB cord.

Core version / branch / commit hash / module tested against: SYNC 3 v3.0
HMI name / version / branch / commit hash / module tested against: SYNC 3 v3.0

Summary

All the crash logs attached to the issue show that the crash occurs when the IAP Control Session attempts to close the output stream. I believe the issue is that the output stream's run loop was created on the lifecycle thread, which means that the lifecycle thread owns the stream. The library then closes the output stream from the main thread. Apple's documentation for streams says You should never attempt to access a scheduled stream from a thread different than the one owning the stream’s run loop.

The IAP Control Session is only created when connecting to legacy units that don't support the multisession protocol. The IAP Data Session opens and closes the streams on the ioThread, which explains why we have not seen this crash on head units that support the multisession protocol.

I changed the code to make sure the iAP control session streams are started on the main thread instead of the lifecycle thread so the streams are started and stopped on the same thread.

Changelog

Bug Fixes
  • Changed the code to make sure the iAP control session streams are started and stopped on the same thread.

CLA

@NicoleYarroch NicoleYarroch self-assigned this Mar 19, 2020
@NicoleYarroch NicoleYarroch added bug A defect in the library transport-iap Relating to the primary IAP transport labels Mar 19, 2020
@NicoleYarroch NicoleYarroch added this to In progress in v6.6.0 via automation Mar 19, 2020
@joeljfischer joeljfischer linked an issue Mar 23, 2020 that may be closed by this pull request
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small change

SmartDeviceLink/SDLIAPControlSession.m Outdated Show resolved Hide resolved
v6.6.0 automation moved this from In progress to Waiting for Review Mar 23, 2020
Co-Authored-By: Joel Fischer <joeljfischer@gmail.com>
v6.6.0 automation moved this from Waiting for Review to Reviewer approved Mar 24, 2020
@joeljfischer joeljfischer merged commit e58fae4 into develop Apr 14, 2020
v6.6.0 automation moved this from Reviewer approved to Done Apr 14, 2020
@joeljfischer joeljfischer deleted the bugfix/issue_1583_iAP_stop_session_crash branch April 14, 2020 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library transport-iap Relating to the primary IAP transport
Projects
No open projects
v6.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

Crash on EAOutputStream close
2 participants