smartdevicelink / sdl_ios

Get your app connected to the 🚙, make your users feel like a 🌟
www.smartdevicelink.com
BSD 3-Clause "New" or "Revised" License
169 stars 105 forks source link

Soft Buttons handlers are never removed #515

Open joeljfischer opened 7 years ago

joeljfischer commented 7 years ago

Bug Report

We store handlers for button events, including SoftButtons. When these handlers are no longer relevant, they should be removed from memory in the iOS library. Currently, however, it appears there is no mechanism in place to do so.

Old SoftButtons are removed from use when a new Show message is sent with SoftButtons. When a new Show message that contains SoftButtons is received, we should remove all old SoftButton handlers.

Reproduction Steps
  1. Connect an app
  2. Send a show with SoftButtons
  3. Send a show with new SoftButtons
Expected Behavior

The old SoftButton handlers are removed from memory.

Observed Behavior

The old SoftButton handlers remain in memory.

OS & Version Information
TODOs
SatbirTanda commented 6 years ago

@joeljfischer was this added to the new milestones? Looks like I'm seeing the old handlers being saved in the customButtonHandlerMap in SDLResponseDispatcher

joeljfischer commented 6 years ago

@SatbirTanda This issue has not yet been fixed and is not currently scheduled for a release. I'm unsure of any good way to fix it, or even if there is a good way, and the downside is not so bad that it needs immediate attention. At some point we will look into it again, but it's not a high priority at the moment.

E-SAITO-TMC commented 4 years ago

@joeljfischer -san, We were able to reproduce this issue with v6.5.0. This is a high priority issue for Toyota. Is it possible to include this in the next release plan?

joeljfischer commented 4 years ago

Hi @E-SAITO-TMC, several attempts were made to solve this issue, but all have failed. I believe that this issue is unfixable. I have applied the wontfix label to it. Please let me know if you think differently, but I don't believe that it's possible to fix this issue with the current library architecture.

E-SAITO-TMC commented 4 years ago

@joeljfischer -san, Thank you for reply. I understand that this issue cannot be fixed. Please let me know if this status changes.

joeljfischer commented 4 years ago

One way this could be improved is to alter the soft button manager to observe the onButtonPress / onButtonEvent notifications itself and handle storing the handlers itself instead of relying on the SDLResponseDispatcher.