~aleteoryx/muditaos

2dc0bc36767a453732b2bdc45d1b5a44cc531708 — mkamonMdt 5 years ago edb144e
[EGD-4850] Fix of statics between USB CDC and ServiceDesktop

Handling queues with static std::string implementation caused
problem of double-free memory on turning off simulator. Applied
solution to the problem is to pass ownership of queued string to
a receiver side.
M module-bsp/board/linux/usb_cdc/usb_cdc.cpp => module-bsp/board/linux/usb_cdc/usb_cdc.cpp +1 -2
@@ 32,7 32,6 @@ namespace bsp
    {
        LOG_INFO("[ServiceDesktop:BSP_Driver] Start reading on fd:%d", fd);
        char inputData[SERIAL_BUFFER_LEN];
        static std::string receiveMsg;

        while (1) {
            if (uxQueueSpacesAvailable(USBReceiveQueue) != 0) {


@@ 61,7 60,7 @@ namespace bsp
                        continue;
                    }
#endif
                    receiveMsg = std::string(inputData, inputData + length);
                    std::string *receiveMsg = new std::string(inputData, inputData + length);
                    xQueueSend(USBReceiveQueue, &receiveMsg, portMAX_DELAY);
                }
                else {

M module-services/service-desktop/ServiceDesktop.cpp => module-services/service-desktop/ServiceDesktop.cpp +1 -1
@@ 42,7 42,7 @@ sys::ReturnCodes ServiceDesktop::InitHandler()
{
    desktopWorker = std::make_unique<WorkerDesktop>(this);
    const bool ret = desktopWorker->init(
        {{sdesktop::RECEIVE_QUEUE_BUFFER_NAME, sizeof(std::string), sdesktop::cdc_queue_len},
        {{sdesktop::RECEIVE_QUEUE_BUFFER_NAME, sizeof(std::string *), sdesktop::cdc_queue_len},
         {sdesktop::SEND_QUEUE_BUFFER_NAME, sizeof(std::string *), sdesktop::cdc_queue_object_size}});

    if (ret == false) {

M module-services/service-desktop/WorkerDesktop.cpp => module-services/service-desktop/WorkerDesktop.cpp +5 -6
@@ 30,7 30,7 @@ bool WorkerDesktop::init(std::list<sys::WorkerQueueInfo> queues)
    Worker::init(queues);

    receiveQueue                         = Worker::getQueueHandleByName(sdesktop::RECEIVE_QUEUE_BUFFER_NAME);
    parserFSM::MessageHandler::sendQueue = Worker::getQueueHandleByName(sdesktop::SEND_QUEUE_BUFFER_NAME);
    parserFSM::MessageHandler::setSendQueueHandle(Worker::getQueueHandleByName(sdesktop::SEND_QUEUE_BUFFER_NAME));

    return (bsp::usbInit(receiveQueue, this) < 0) ? false : true;
}


@@ 58,16 58,17 @@ bool WorkerDesktop::handleMessage(uint32_t queueID)

    LOG_INFO("handleMessage received data from queue: %s", qname.c_str());

    static std::string *sendMsg = nullptr;
    static std::string receivedMsg;
    if (qname == sdesktop::RECEIVE_QUEUE_BUFFER_NAME) {
        std::string *receivedMsg = nullptr;
        if (!queue->Dequeue(&receivedMsg, 0)) {
            LOG_ERROR("handleMessage failed to receive from \"%s\"", sdesktop::RECEIVE_QUEUE_BUFFER_NAME);
            return false;
        }
        parser.processMessage(receivedMsg);
        parser.processMessage(std::move(*receivedMsg));
        delete receivedMsg;
    }
    else if (qname == sdesktop::SEND_QUEUE_BUFFER_NAME) {
        std::string *sendMsg = nullptr;
        if (!queue->Dequeue(&sendMsg, 0)) {
            LOG_ERROR("handleMessage xQueueReceive failed for %s.", sdesktop::SEND_QUEUE_BUFFER_NAME);
            return false;


@@ 75,9 76,7 @@ bool WorkerDesktop::handleMessage(uint32_t queueID)

        LOG_DEBUG("handeMessage sending %d bytes using usbCDCSend", static_cast<unsigned int>(sendMsg->length()));
        bsp::usbCDCSend(sendMsg);

        delete sendMsg;
        sendMsg = nullptr;
    }
    else {
        LOG_INFO("handeMessage got message on an unhandled queue");

M module-services/service-desktop/parser/MessageHandler.cpp => module-services/service-desktop/parser/MessageHandler.cpp +3 -3
@@ 23,7 23,7 @@ using namespace parserFSM;

xQueueHandle MessageHandler::sendQueue;

MessageHandler::MessageHandler(std::string &message, sys::Service *OwnerService) : OwnerServicePtr(OwnerService)
MessageHandler::MessageHandler(const std::string &message, sys::Service *OwnerService) : OwnerServicePtr(OwnerService)
{
    try {
        messageJson = json11::Json::parse(message, JsonErrorMsg);


@@ 53,10 53,10 @@ void MessageHandler::processMessage()
    }
}

void MessageHandler::putToSendQueue(const std::string msg)
void MessageHandler::putToSendQueue(const std::string &msg)
{
    if (uxQueueSpacesAvailable(sendQueue) != 0) {
        auto *responseString = new std::string(msg);
        auto responseString = new std::string(msg);
        xQueueSend(sendQueue, &responseString, portMAX_DELAY);
    }
}

M module-services/service-desktop/parser/MessageHandler.hpp => module-services/service-desktop/parser/MessageHandler.hpp +14 -11
@@ 24,29 24,32 @@ namespace parserFSM
{
    class MessageHandler
    {
        static xQueueHandle sendQueue;

        json11::Json messageJson;
        std::string JsonErrorMsg;
        sys::Service *OwnerServicePtr = nullptr;

      public:
        MessageHandler(std::string &message, sys::Service *OwnerService);
        static xQueueHandle sendQueue;
        MessageHandler(const std::string &message, sys::Service *OwnerService);

        bool isJSONNull()
        [[nodiscard]] auto isJSONNull() const -> bool
        {
            return messageJson.is_null();
        };
        bool isValid()
        [[nodiscard]] auto isValid() const noexcept -> bool
        {
            return JsonErrorMsg.empty();
        }
        std::string &getErrorString()
        [[nodiscard]] auto getErrorString() const -> const std::string &
        {
            return JsonErrorMsg;
        };
        void processMessage();
        static void putToSendQueue(const std::string msg);

      private:
        json11::Json messageJson;
        std::string JsonErrorMsg;
        sys::Service *OwnerServicePtr = nullptr;
        static void putToSendQueue(const std::string &msg);
        static void setSendQueueHandle(xQueueHandle handle)
        {
            sendQueue = handle;
        }
    };
} // namespace parserFSM

M module-services/service-desktop/parser/ParserFSM.cpp => module-services/service-desktop/parser/ParserFSM.cpp +23 -23
@@ 28,10 28,10 @@ using namespace parserFSM;
StateMachine::StateMachine(sys::Service *OwnerService) : OwnerServicePtr(OwnerService)
{}

void StateMachine::processMessage(std::string &msg)
void StateMachine::processMessage(std::string &&msg)
{
    receivedMsgPtr = &msg;
    LOG_DEBUG("Msg: %s", receivedMsgPtr->c_str());
    receivedMsg = std::move(msg);
    LOG_DEBUG("Msg: %s", receivedMsg.c_str());

    switch (state) {
    case State::ReceivedPayload:


@@ 58,20 58,20 @@ void StateMachine::parseHeader()
    header.clear();
    payloadLength = 0;

    auto messageStart = receivedMsgPtr->find(message::endpointChar);
    auto messageStart = receivedMsg.find(message::endpointChar);
    if (messageStart == std::string::npos) {
        LOG_ERROR("This is not a valid endpoint message! Type=%c", receivedMsgPtr->at(0));
        LOG_ERROR("This is not a valid endpoint message! Type=%c", receivedMsg.at(0));
        return;
    }

    if (receivedMsgPtr->size() < message::size_header) // header divided in few parts
    if (receivedMsg.size() < message::size_header) // header divided in few parts
    {
        state = State::ReceivedPartialHeader;
        header.append(*receivedMsgPtr); // append to whole header string
        header.append(receivedMsg); // append to whole header string
        return;
    }

    header        = message::getHeader(*receivedMsgPtr);
    header        = message::getHeader(receivedMsg);
    payloadLength = message::calcPayloadLength(header);
    if (payloadLength == 0) // failed to obtain payload length from msg
    {


@@ 82,7 82,7 @@ void StateMachine::parseHeader()

    LOG_DEBUG("Payload length: %lu", payloadLength);

    message::removeHeader(*receivedMsgPtr);
    message::removeHeader(receivedMsg);
    parseNewMessage();
}



@@ 91,39 91,39 @@ void StateMachine::parsePartialHeader()
    auto previousHeaderLength = header.size();
    auto missingHeaderLength  = message::size_header - previousHeaderLength;

    if (receivedMsgPtr->size() >= missingHeaderLength) // rest of the message is here
    if (receivedMsg.size() >= missingHeaderLength) // rest of the message is here
    {
        header.append(receivedMsgPtr->substr(0, missingHeaderLength));
        header.append(receivedMsg.substr(0, missingHeaderLength));
        LOG_DEBUG("Header: %s\n", header.c_str());
        payloadLength = message::calcPayloadLength(header);

        LOG_DEBUG("Payload length: %lu\n", payloadLength);
        message::eraseFront(*receivedMsgPtr, missingHeaderLength);
        message::eraseFront(receivedMsg, missingHeaderLength);

        parseNewMessage();
    }
    else // the message is even longer :(
    {
        header.append(*receivedMsgPtr);
        header.append(receivedMsg);
    }
}

void StateMachine::parseNewMessage()
{
    if (receivedMsgPtr->size() >= payloadLength) {
    if (receivedMsg.size() >= payloadLength) {

        payload = message::extractPayload(*receivedMsgPtr, payloadLength);
        payload = message::extractPayload(receivedMsg, payloadLength);

        parsePayload();

        if (receivedMsgPtr->size() > payloadLength) { // contains part of new header
            message::eraseFront(*receivedMsgPtr, payloadLength);
        if (receivedMsg.size() > payloadLength) { // contains part of new header
            message::eraseFront(receivedMsg, payloadLength);
            parseHeader();
        }
    }
    else // message divided in 2 or more packets
    {
        payload = receivedMsgPtr->substr(0, std::string::npos); // take rest of the message
        payload = receivedMsg.substr(0, std::string::npos); // take rest of the message
        state   = State::ReceivedPartialPayload;
    }
}


@@ 133,20 133,20 @@ void StateMachine::parsePartialMessage()
    auto previousPayloadLength = payload.size();
    auto missingPayloadLength  = payloadLength - previousPayloadLength;

    if (receivedMsgPtr->size() >= missingPayloadLength) // rest of the message is here
    if (receivedMsg.size() >= missingPayloadLength) // rest of the message is here
    {
        payload.append(message::extractPayload(*receivedMsgPtr, missingPayloadLength));
        payload.append(message::extractPayload(receivedMsg, missingPayloadLength));

        parsePayload();

        if (receivedMsgPtr->size() > missingPayloadLength) {
            message::eraseFront(*receivedMsgPtr, missingPayloadLength);
        if (receivedMsg.size() > missingPayloadLength) {
            message::eraseFront(receivedMsg, missingPayloadLength);
            parseHeader();
        }
    }
    else // the message is even longer
    {
        payload.append(*receivedMsgPtr);
        payload.append(receivedMsg);
    }
}


M module-services/service-desktop/parser/ParserFSM.hpp => module-services/service-desktop/parser/ParserFSM.hpp +6 -6
@@ 23,21 23,21 @@ namespace parserFSM
    class StateMachine
    {
      public:
        StateMachine(sys::Service *OwnerService);
        void processMessage(std::string &msg);
        State getCurrentState()
        explicit StateMachine(sys::Service *OwnerService);
        void processMessage(std::string &&msg);
        [[nodiscard]] auto getCurrentState() const noexcept -> State
        {
            return state;
        };

        void setState(const parserFSM::State newState)
        void setState(const parserFSM::State newState) noexcept
        {
            state = newState;
        }

      private:
        std::string *receivedMsgPtr = nullptr;
        parserFSM::State state      = State::NoMsg;
        std::string receivedMsg;
        parserFSM::State state = State::NoMsg;
        std::string payload;
        std::string header;
        unsigned long payloadLength   = 0;

M module-services/service-desktop/tests/unittest.cpp => module-services/service-desktop/tests/unittest.cpp +25 -21
@@ 68,76 68,80 @@ using namespace parserFSM;
TEST_CASE("Parser Test")
{
    StateMachine parser(nullptr);
    std::string testMessage("#00000");

    SECTION("Parse message with divided header and payload")
    {
        parser.processMessage(testMessage);
        std::string testMessage("#00000");
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPartialHeader);

        testMessage = R"(0050{"endpo)";
        parser.processMessage(testMessage);
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPartialPayload);

        testMessage = R"(int":1, "method":1, "body":{"test":"test"}})";
        parser.processMessage(testMessage);
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPayload);
    }

    SECTION("Parse whole message")
    {
        testMessage = R"(#000000050{"endpoint":1, "method":1, "body":{"test":"test"}})";
        parser.processMessage(testMessage);
        std::string testMessage = R"(#000000050{"endpoint":1, "method":1, "body":{"test":"test"}})";
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPayload);
    }

    SECTION("Parse message with start char detached from mesage")
    {
        testMessage = R"(#)";
        parser.processMessage(testMessage);
        std::string testMessage = R"(#)";
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPartialHeader);

        testMessage = R"(000000050{"en)";
        parser.processMessage(testMessage);
        testMessage = R"(000000050{"en)";
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPartialPayload);

        testMessage = R"(dpoint":1, "method":1, "body":{"test":"test"}})";
        parser.processMessage(testMessage);
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPayload);
    }

    SECTION("Parse message with beginning of another one")
    {
        testMessage = R"(#000000050{"endpoint":1, "method":1, "body":{"test":"test"}}#000000050{"end)";
        parser.processMessage(testMessage);
        std::string testMessage = R"(#000000050{"endpoint":1, "method":1, "body":{"test":"test"}}#000000050{"end)";
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPartialPayload);

        testMessage = R"(point":1, "method":1, "body":{"test":"test"}})";
        parser.processMessage(testMessage);
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::ReceivedPayload);
    }
    SECTION("Parse junk message")
    {
        testMessage = R"({"address": "6 Czeczota St.\n02600 Warsaw"})";
        parser.processMessage(testMessage);
        std::string testMessage = R"({"address": "6 Czeczota St.\n02600 Warsaw"})";
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::NoMsg);
    }
    SECTION("Parse message with incorrect header length")
    {
        testMessage = R"(#000000072{"endpoint":7, "method":2, "uuid":3, "body":{"threadID":1,"unread":false}})";
        parser.processMessage(testMessage);
        std::string testMessage =
            R"(#000000072{"endpoint":7, "method":2, "uuid":3, "body":{"threadID":1,"unread":false}})";
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::NoMsg);
    }
    SECTION("Parse message with damaged json ")
    {
        testMessage = R"(#000000074{"endpoint":7, "method":2, "uuid":3, "bo}}dy":{"threadID":1,"unread":false)";
        parser.processMessage(testMessage);
        std::string testMessage =
            R"(#000000074{"endpoint":7, "method":2, "uuid":3, "bo}}dy":{"threadID":1,"unread":false)";
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::NoMsg);
    }
    SECTION("Parse message with damaged json and incorrect header length")
    {
        testMessage = R"(#000000072{"endpoint":7, "method":2, "uuid":3, "bo}}dy":{"threadID":1,"unread":false)";
        parser.processMessage(testMessage);
        std::string testMessage =
            R"(#000000072{"endpoint":7, "method":2, "uuid":3, "bo}}dy":{"threadID":1,"unread":false)";
        parser.processMessage(std::move(testMessage));
        REQUIRE(parser.getCurrentState() == State::NoMsg);
    }
}