Assertion IsArray() failed when SampleApp tries to connect to GUI

marcardenas commented 4 years ago

IMPORTANT: Before you create an issue, please take a look at our Issue Reporting Guide.

Briefly summarize your issue:

Running SampleApp along with the GUI server results in a failed assertion after connecting to the GUI. The detailed assertion is

SampleApp: /home/ubuntu/marcelo/sdk_folder/alexa-smart-screen-sdk/modules/Alexa/ThirdParty/rapidjson/rapidjson-1.1.0/include/rapidjson/document.h:1505: rapidjson::GenericValue<Encoding, Allocator>* rapidjson::GenericValue<Encoding, Allocator>::Begin() [with Encoding = rapidjson::UTF8<>; Allocator = rapidjson::MemoryPoolAllocator<>; rapidjson::GenericValue<Encoding, Allocator>::ValueIterator = rapidjson::GenericValue<rapidjson::UTF8<> >*]: Assertion 'IsArray()' failed.

What is the expected behavior?

SampleApp should indicate that is on IDLE state (client side) and the GUI should change to "Press and hold 'A' to speak" (server side), allowing me to perform a request.

What behavior are you observing?

GUI changes from "Configuring device..." to "Press and hold 'A' to speak". SampleApp connects to the GUI, but then an assertion fails, closing the application. The log is attached on this section, but application never passes of ContextManager:buildContextIgnored:namespace=AudioActivityTracker,name=ActivityState log message.

log.log

Provide the steps to reproduce the issue, if applicable:

Open index.html file with a supported browser. In this case, it was used minimal, a qtwebengine based web broser (https://doc.qt.io/qt-5/qtwebengine-webengine-minimal-example.html). This command should open the GUI in the screen ./minimal ~/sdk_folder/ss-build/modules/GUI/index.html --no-sandbox
Run SampleApp .~/sdk_folder/ss-build/modules/Alexa/SampleApp/src/SampleApp -C /path/to/AlexaClientSDKConfig.json -C /path/to/SmartScreenSDKConfig.json -L DEBUG9

Tell us about your environment:

I've built the application using STM32MP1 Developer Package, a toolchain to cross-compile applications for the MP1 (https://wiki.st.com/stm32mpu/wiki/STM32MP1_Developer_Package#Installing_the_SDK). All the libraries and binaries were deployed then to the MP1 using SCP.

NOTE: I've built avs-device-sdk and tested SampleApp successfully before on the same board

What version of the AVS Device SDK are you using?

v1.15 (it's the only supported for alexa-smart-screen-sdk)

Tell us what hardware you're using:

[ ] Desktop / Laptop
[ ] Raspberry Pi
[x] Other - tell us more: ST's STM32MP157 DK2 board (ARM A7 32 bits processor + M4 MPU)

Tell us about your OS (Type & version):

[ ] Linux
[ ] MacOS
[ ] Raspbian Stretch
[ ] Raspbian Jessy
[x] Other - tell us more: Custom Linux distro based on OpenSTLinux. This distro was built using Yocto + OpenEmbedded (Thud branch layers)

skovba commented 4 years ago

Hi @marcardenas,

Unfortunately we do not support this platform and browser at the moment, so will not be able to reproduce. Can you provide configuration you are using? JSON handling is unlikely to fail in websocket communication layer, so it's a first place I can think of.

marcardenas commented 4 years ago

Hi @skovba

I've performed a debug at the code and found that the error is generated at line 323 of avs-device-sdk/ContextManager/src/ContextManager.cpp (avs-device-sdk v1.15 @ https://github.com/alexa/avs-device-sdk/blob/v1.15/ContextManager/src/ContextManager.cpp). The error always occur just right after

2020-02-20 18:32:41.577 [ 27] 9 ContextManager:updateStateLocked:action=updatedState,namespace=Notifications,name=IndicatorState
2020-02-20 18:32:41.578 [ 34] 9 ContextManager:buildContextIgnored:namespace=Alexa.Presentation.APL,name=RenderedDocumentState
2020-02-20 18:32:41.578 [ 34] 9 ContextManager:buildContextIgnored:namespace=VisualActivityTracker,name=ActivityState
2020-02-20 18:32:41.579 [ 34] 9 ContextManager:buildContextIgnored:namespace=AudioActivityTracker,name=ActivityState

About configuration, what kind of details do you need?

Thanks for your help!

skovba commented 4 years ago

Hi @marcardenas Json config that you use for smart-screen SDK configuration will be great. Also if you can - full stack trace will be helpful.

marcardenas commented 4 years ago

Hi @skovba

I've tested with all the SDK configurations samples available at modules folder, and none of them worked. Here is the complete log of the application. If it is not enough, how can i generate the full stack trace?

https://github.com/alexa/alexa-smart-screen-sdk/files/4232412/log.log

lynx-arul commented 4 years ago

Hi @marcardenas you can use pstack to capture the full stack trace. ref: https://linux.die.net/man/1/pstack

marcardenas commented 4 years ago

Hi guys!

I tried to use pstack on our board, but unfortunately it is not compatible. However, i've managed to capture the backtrace using gdb/server.

SS-backtrace_03-11-20_18:00.log

VaruPan commented 4 years ago

Hi @marcardenas,

Thanks for sharing the backtrace, unfortunately it is not complete. Can you provide the full trace logs? Could you also try running AVS Device SDK SampleApp to verify that device SDK is installed correctly.

marcardenas commented 4 years ago

Hi @VaruPan

First of all, AVS Device SDK SampleApp works okay on our target device (v1.15). Next, i've got two different backtraces: one backtrace is obtained by running SmartScreen SampleApp first and after that opening the web browser(#1), and the other backtrace is obtained by opening web browser first and then opening SampleApp (#2)

#1-SS-backtrace_03-17-20_00:15.log #2-SS-backtrace_03-17-20_00:15.log

VaruPan commented 4 years ago

Hi @marcardenas,

Thanks for the backtrace, this is helpful. Can you also share the output of JSON payload received in handleDeviceWindowState? You can add a print statement here:

https://github.com/alexa/alexa-smart-screen-sdk/blob/master/modules/Alexa/SampleApp/src/GUI/GUIClient.cpp#L625

marcardenas commented 4 years ago

@VaruPan this is the output print of the payload

{"defaultWindowId":"tvFullscreen","instances":[{"id":"tvFullscreen","templateId":"tvFullscreen","token":null,"configuration":{"interactionMode":"tv","sizeConfigurationId":"fullscreen"}},{"id":"tvOverlayLandscape","templateId":"tvOverlayLandscape","token":null,"configuration":{"interactionMode":"tv_overlay","sizeConfigurationId":"landscapePanel"}}]}

wjennings commented 4 years ago

I ran into the same problem. Originally, I built SmartScreen 2.0.1 on my Ubuntu machine, and it worked fine.

When I tried to port it to one of our embedded clients, I got the strange rapidjson errors. note, avs-device-sdk ran fine before I added smart-screen.

I finally, got it working, but I'm not at all comfortable with the solution. I added a flag for both avs-device-sdk and avs-smartscreen: -DCMAKE_CXX_FLAGS="-DRAPIDJSON_PARSE_DEFAULT_FLAGS=36 (which sets the Comments (32) and iterative (4) flags)

That seemed to fix some issues, but, I still was getting the issue on a different platform in the GuiClient. I ended up, in GUIClient::sendGuiConfiguration(), doing this:

//auto message = messages::GuiConfigurationMessage(visualCharacteristics.serialize(), appConfig.serialize()); //sendMessage(message); std::string messageStr = "{\"type\":\"guiConfiguration\",\"payload\":{\"" + std::string(GUI_MSG_VISUALCHARACTERISTICS_TAG) + "\":" + visualCharacteristics.serialize() + ",\"" + std::string(GUI_MSG_APPCONFIG_TAG) + "\":" + appConfig.serialize() + "}}"; writeMessage(messageStr);

I'm running now on my platforms, but I'm worried I didn't really solve the problem. I looks like a memory corruption issue, or stack issue, but I couldn't find the root cause.

wjennings commented 4 years ago

FYI, I noticed there's a difference in rapidjson's from the smart screen, and avs-device-sdk.

Here is a diff:

diff -r rapidjson/rapidjson-1.1.0/include/rapidjson/internal/biginteger.h 1.15-r0/git/ThirdParty/rapidjson/rapidjson-1.1.0/include/rapidjson/internal/biginteger.h
136c136
<             std::memmove(digits_ + offset, digits_, count_ * sizeof(Type));
---
>             std::memmove(&digits_[count_ - 1 + offset], &digits_[count_ - 1], count_ * sizeof(Type));
diff -r rapidjson/rapidjson-1.1.0/include/rapidjson/rapidjson.h 1.15-r0/git/ThirdParty/rapidjson/rapidjson-1.1.0/include/rapidjson/rapidjson.h
267c267,268
<     Some machines require strict data alignment. The default is 8 bytes.
---
>     Some machines require strict data alignment. Currently the default uses 4 bytes
>     alignment on 32-bit platforms and 8 bytes alignment for 64-bit platforms.
271c272,276
< #define RAPIDJSON_ALIGN(x) (((x) + static_cast<size_t>(7u)) & ~static_cast<size_t>(7u))
---
> #if RAPIDJSON_64BIT == 1
> #define RAPIDJSON_ALIGN(x) (((x) + static_cast<uint64_t>(7u)) & ~static_cast<uint64_t>(7u))
> #else
> #define RAPIDJSON_ALIGN(x) (((x) + 3u) & ~3u)
> #endif
diff -r rapidjson/rapidjson-1.1.0/include/rapidjson/schema.h 1.15-r0/git/ThirdParty/rapidjson/rapidjson-1.1.0/include/rapidjson/schema.h

1020d1019
<             RegexType *r = static_cast<RegexType*>(allocator_->Malloc(sizeof(RegexType)));
1022c1021
<                 return new (r) RegexType(value.GetString(), std::size_t(value.GetStringLength()), std::regex_constants::ECMAScript);
---
>                 return new (allocator_->Malloc(sizeof(RegexType))) RegexType(value.GetString(), std::size_t(value.GetStringLength()), std::regex_constants::ECMAScript);
1025d1023
<                 AllocatorType::Free(r);
diff -r rapidjson/rapidjson-1.1.0/test/unittest/allocatorstest.cpp 1.15-r0/git/ThirdParty/rapidjson/rapidjson-1.1.0/test/unittest/allocatorstest.cpp
66,73c66,72
<     if (sizeof(size_t) >= 8) {
<         EXPECT_EQ(RAPIDJSON_UINT64_C2(0x00000000, 0x00000000), RAPIDJSON_ALIGN(0));
<         for (uint64_t i = 1; i < 8; i++) {
<             EXPECT_EQ(RAPIDJSON_UINT64_C2(0x00000000, 0x00000008), RAPIDJSON_ALIGN(i));
<             EXPECT_EQ(RAPIDJSON_UINT64_C2(0x00000000, 0x00000010), RAPIDJSON_ALIGN(RAPIDJSON_UINT64_C2(0x00000000, 0x00000008) + i));
<             EXPECT_EQ(RAPIDJSON_UINT64_C2(0x00000001, 0x00000000), RAPIDJSON_ALIGN(RAPIDJSON_UINT64_C2(0x00000000, 0xFFFFFFF8) + i));
<             EXPECT_EQ(RAPIDJSON_UINT64_C2(0xFFFFFFFF, 0xFFFFFFF8), RAPIDJSON_ALIGN(RAPIDJSON_UINT64_C2(0xFFFFFFFF, 0xFFFFFFF0) + i));
<         }
---
> #if RAPIDJSON_64BIT == 1
>     EXPECT_EQ(RAPIDJSON_UINT64_C2(0x00000000, 0x00000000), RAPIDJSON_ALIGN(0));
>     for (uint64_t i = 1; i < 8; i++) {
>         EXPECT_EQ(RAPIDJSON_UINT64_C2(0x00000000, 0x00000008), RAPIDJSON_ALIGN(i));
>         EXPECT_EQ(RAPIDJSON_UINT64_C2(0x00000000, 0x00000010), RAPIDJSON_ALIGN(RAPIDJSON_UINT64_C2(0x00000000, 0x00000008) + i));
>         EXPECT_EQ(RAPIDJSON_UINT64_C2(0x00000001, 0x00000000), RAPIDJSON_ALIGN(RAPIDJSON_UINT64_C2(0x00000000, 0xFFFFFFF8) + i));
>         EXPECT_EQ(RAPIDJSON_UINT64_C2(0xFFFFFFFF, 0xFFFFFFF8), RAPIDJSON_ALIGN(RAPIDJSON_UINT64_C2(0xFFFFFFFF, 0xFFFFFFF0) + i));
75c74
< 
---
> #else
77,79c76,80
<     for (uint32_t i = 1; i < 8; i++) {
<         EXPECT_EQ(8u, RAPIDJSON_ALIGN(i));
<         EXPECT_EQ(0xFFFFFFF8u, RAPIDJSON_ALIGN(0xFFFFFFF0u + i));
---
>     for (uint32_t i = 1; i < 4; i++) {
>         EXPECT_EQ(4u, RAPIDJSON_ALIGN(i));
>         EXPECT_EQ(8u, RAPIDJSON_ALIGN(4u + i));
>         EXPECT_EQ(0xFFFFFFF8u, RAPIDJSON_ALIGN(0xFFFFFFF4u + i));
>         EXPECT_EQ(0xFFFFFFFCu, RAPIDJSON_ALIGN(0xFFFFFFF8u + i));
80a82
> #endif

Those should definitely be the same!

marcardenas commented 4 years ago

Hi all! Thanks for your support.

I've tested the following:

CMake flag did not worked for me. I've built both projects with the rapidjson flag at https://github.com/alexa/alexa-smart-screen-sdk/issues/17#issuecomment-606215033 but issue still on board.
Commenting GuiClient lines avoids the assert generation, but it gets stuck on configuring the device on browser (https://github.com/alexa/alexa-smart-screen-sdk/issues/17#issuecomment-606215033)
Tested replacing rapidjson from SmartScreen with AVS SDK and viceversa. No resolution.

powj commented 4 years ago

Hi @marcardenas,

Thanks for following up with this information. I've identified a potential issue with how JSON documents are created for the messages we send and have attached a patch which which should address this. Can you test this and let us know if it resolves your issue?

Thanks MessagesAllocPatch.txt

VaruPan commented 4 years ago

Closing this due to inactivity. Please feel free to re-open if you encounter further problems. Thank you for your contribution to the Alexa Smart Screen SDK!

wjennings commented 4 years ago

I removed my workarounds, added the patch, but I still get the Segmentation Fault.

FWIW my CPU is also an ARM 64-bit. I assume the latest smart-screen sdk has been tested on one? (I supposed RPi's are ARM 64).

marcardenas commented 4 years ago

Hi @powj

I've applied the patch on GuiClientMessage.h. However, i'm still getting the same error in the same spot.

marcardenas commented 4 years ago

Hi all! I've patched the issue by commenting lines 272-274-275-276 of avs-device-sdk/ThirdParty/rapidjson/rapidjson-1.1.0/include/rapidjson.h. The code should look like

#ifndef RAPIDJSON_ALIGN
//#if RAPIDJSON_64BIT == 1
#define RAPIDJSON_ALIGN(x) (((x) + static_cast<uint64_t>(7u)) & ~static_cast<uint64_t>(7u))
//#else
//#define RAPIDJSON_ALIGN(x) (((x) + 3u) & ~3u)
//#endif
#endif

Now it works on my machine

wjennings commented 4 years ago

So, the real fix would be to include a -DRAPIDJSON_64BIT=1 to the compiler flags, instead of changing the rapidjson header file, right? I'll try it later on my build

I wondered if it was a 64-bit issue on some processors, hopefully this fixes it for us all.

wjennings commented 4 years ago

Hi all! I've patched the issue by commenting lines 272-274-275-276 of avs-device-sdk/ThirdParty/rapidjson/rapidjson-1.1.0/include/rapidjson.h. The code should look like
#ifndef RAPIDJSON_ALIGN
//#if RAPIDJSON_64BIT == 1
#define RAPIDJSON_ALIGN(x) (((x) + static_cast<uint64_t>(7u)) & ~static_cast<uint64_t>(7u))
//#else
//#define RAPIDJSON_ALIGN(x) (((x) + 3u) & ~3u)
//#endif
#endif
Now it works on my machine

If you tried the smart-screen-sdk version of rapidjson, that is what is used by default:

< #define RAPIDJSON_ALIGN(x) (((x) + static_cast<size_t>(7u)) & ~static_cast<size_t>(7u))
---
> #if RAPIDJSON_64BIT == 1
> #define RAPIDJSON_ALIGN(x) (((x) + static_cast<uint64_t>(7u)) & ~static_cast<uint64_t>(7u))
> #else
> #define RAPIDJSON_ALIGN(x) (((x) + 3u) & ~3u)
> #endif

What I'm saying is, if you build both smart-screen and device-sdk with the smart-screen's rapidjson, then you should have by default been using that marco. So it doesn't quite fit with what you said earlier you tried. I'd be worried about that fix.

I am, by default, using the smart-screen-sdk version of rapidjson for both, which ensures it uses the RAPIDJSON_ALIGN as defined, but I still have to add my strange workarounds to get it to work.

So, I'm not sure that is the real solution. We're just getting it working with hacks IMO, and needs to be fixed.

nguymi91 commented 3 years ago

Closing as this is being tracked internally and the fix will in our future releases.

alexa / alexa-smart-screen-sdk