Skip to content
This repository has been archived by the owner on Jan 16, 2024. It is now read-only.

MessageRouter::disconnectAllTransportsLocked flow leads to erase while iterating transports vector #8

Closed
jshslt opened this issue May 17, 2017 · 2 comments
Assignees
Milestone

Comments

@jshslt
Copy link

jshslt commented May 17, 2017

MessageRouter::disconnectAllTransportsLocked uses a range-based for to iterate m_transports. this conflicts with the transport disconnect flow, which leads back to MessageRouter::onDisconnected and if still connected, an erase-remove on m_transports. This invalidates the outer iterator used by disconnectAllTransportsLocked, and can lead to segfault -

#0  0x76e6609c in alexaClientSDK::acl::MessageRouter::disconnectAllTransportsLocked (this=0xa4c7b8, reason=alexaClientSDK::acl::SERVER_ENDPOINT_CHANGED) at ../../../src/ACL/src/Transport/MessageRouter.cpp:177
#1  0x76e65834 in alexaClientSDK::acl::MessageRouter::setAVSEndpoint (this=0xa4c7b8, avsEndpoint=...) at ../../../src/ACL/src/Transport/MessageRouter.cpp:91
#2  0x0007a90c in AVS2Service::setEndpointLocked (this=0xad2140) at voiceservice_avs2.cxx:576
#3  0x0007b208 in AVS2Service::handleSetEndpointDirective (this=this@entry=0xad2140, directive=...) at voiceservice_avs2.cxx:881
#4  0x0007fc48 in AVS2Service::receive (this=0xad2140, msg=...) at voiceservice_avs2.cxx:791
#5  0x76e65cec in alexaClientSDK::acl::MessageRouter::__lambda9::operator() (__closure=0xb1b96c) at ../../../src/ACL/src/Transport/MessageRouter.cpp:158

our local solution was to set m_connectionStatus = ConnectionStatus::DISCONNECTED before iterating the transports and calling disconnect() on each.

@sanjayrd
Copy link
Contributor

Thank you for pointing this out @jshslt. We plan on addressing this issue in either our 0.4.1 release, which we plan on being a patch update, or the 0.5 release, which should be a general improvements update. Both of these should be coming in the next few weeks.

@JamieMeyers JamieMeyers added the bug label Jun 2, 2017
@yugoren yugoren added the ACL label Jun 2, 2017
@scotthea-amazon scotthea-amazon self-assigned this Jun 8, 2017
kjkh pushed a commit that referenced this issue Jun 9, 2017
Changes in this update
-Implemented Sensory wake word detector functionality
-Removed the need for a std::recursive_mutex in MessageRouter
-Added AIP unit test
-Added handleDirectiveImmediately functionality to SpeechSynthesizer
-Added memory profiles for:
AIP
SpeechSynthesizer
ContextManager
AVSUtils
AVSCommon
-Bug fix for MultipartParser.h compiler warning
-Suppression of sensitive log data even in debug builds. Use cmake parameter -DACSDK_EMIT_SENSITIVE_LOGS=ON to allow logging of sensitive information in DEBUG builds
-Fix crash in ACL when attempting to use more than 10 streams
-Updated MediaPlayer to use autoaudiosink instead of requiring pulseaudio
-Updated MediaPlayer build to suppport local builds of GStreamer
-Fixes for the following Github issues:
#5
#8
#9
#10
#17
#24
@scotthea-amazon
Copy link
Contributor

A fix for this has been pushed in version 0.4.1.

@JamieMeyers JamieMeyers added this to the 0.4.1 milestone Jun 13, 2017
@adeb adeb mentioned this issue Apr 8, 2018
6 tasks
@xuqifu xuqifu mentioned this issue Jul 12, 2018
6 tasks
Guillaume0477 pushed a commit to Guillaume0477/avs-device-sdk that referenced this issue Sep 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants