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

CCB: Fix dangerous work with app commands #3170

Closed
LitvinenkoIra opened this issue Dec 6, 2019 · 6 comments
Closed

CCB: Fix dangerous work with app commands #3170

LitvinenkoIra opened this issue Dec 6, 2019 · 6 comments

Comments

@LitvinenkoIra
Copy link
Contributor

LitvinenkoIra commented Dec 6, 2019

Bug Report

Crash in AttachSchema SDL

Detailed analysis:

There was related to a thread safety violation while working with application internal commands
array. This array is a thread-safe itself as it is protected with mutexes, however FindCommand()
function is returning a raw pointer to an internal array element so the external component is able to get access to this element at any time even without locking the mutex. Core crash which was found in DeleteCommand request instance is happening by exactly the same reason -
DeleteCommand is using a raw pointer to find the command, however, this command might be destroyed from another thread at this point in time. As a result, working thread can access to a destroyed object by pointer. As a quick solution, each FindCommand() can be followed
by a command accessor which prevents unexpected command destruction. Working thread will use the temporary copy of command, but not command itself. However, the better solution would be to change the design and to avoid using of the raw pointers.

Reproduction Steps
  1. Start SDL, HMI, connect Mobile
  2. Register app
  3. Add some commands and then send DeleteCommand RPC
  4. Observe behavior
Expected Behavior

No crash

Observed Behavior

SDL crashed
bt.txt

OS & Version Information

@E-SAITO-TMC
Copy link

E-SAITO-TMC commented Apr 17, 2020

@joeygrover -san @theresalech -san This issue is a top priority for Toyota. Can the reproduction steps described actually reproduce this issue? Also, does this issue occur in the latest version?

@theresalech
Copy link
Contributor

Hi @E-SAITO-TMC, we have documented that this is priority for Toyota, and will include for Steering Committee review and vote when planning the contents for the Q4 Core release. This planning will take place in June.
@LitvinenkoIra, can you please advise if the Luxoft team is seeing this issue in Core 6.1?

@theresalech theresalech added the Waiting for Feedback Maintainers are waiting for feedback from an author or contributor label Apr 23, 2020
@LuxoftAKutsan
Copy link
Contributor

@theresalech
Issue reproduced on SYNC version of SDL.
On open source version of SDL issue may be non reproducible.
Script will be created.
Fix from SYNC ported to open : #3375

@theresalech
Copy link
Contributor

@LuxoftAKutsan, thank you! We will wait for confirmation if the issue is reproducible in the open source based on the findings of your script.

@theresalech theresalech removed the Waiting for Feedback Maintainers are waiting for feedback from an author or contributor label Jun 24, 2020
@dboltovskyi
Copy link
Contributor

@theresalech Please notice the following findings:

Script for provided use case has been created: 3170.lua.tar.gz

Issue was not reproduced on latest SDL develop - neither with fix nor without it.

There could be several reasons why issue is not reproduced in open source environment :

  • Timings on Mobile\HMI communication may be different on CI/developer workstation and real hardware
  • QNX platform specific issues may trigger this case
  • Different CPU resources available on HU, and developer/CI.

Anyway script can be used to check use case on real hardware with Remote ATF feature.
And we think proposed fix should be included since it solves 'potential' problem.

@theresalech
Copy link
Contributor

@dboltovskyi, thank you for the additional information. We'll await Luxoft's confirmation of PR #3375 being ready for Livio review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants