~aleteoryx/muditaos

8c2b5aa7b08f1bbd6d8497e0df3860cfba790d86 — Dawid Wojtas 2 years ago 1376229
[BH-1624] Fix shutdown procedure

In some cases, the system wasn't able to turn off
because the GUI service got stuck. The device
was still working in the background. The cause
was an empty queue in DrawCommandQueue which
hang the GUI worker.

The interface was modified and synchronization
mechanism was removed.
The thread no longer waits in dequeue().

Also changed the worker to close in the right
way the logger worker.
M module-services/service-gui/DrawCommandsQueue.cpp => module-services/service-gui/DrawCommandsQueue.cpp +12 -14
@@ 1,18 1,16 @@
// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved.
// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved.
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#include "DrawCommandsQueue.hpp"

#include <log/log.hpp>

#include <algorithm>

namespace service::gui
{
    namespace
    {
        constexpr std::chrono::milliseconds WaitTimeout{5000};
    } // namespace

    DrawCommandsQueue::DrawCommandsQueue(std::size_t expectedSize,
                                         std::unique_ptr<SynchronizationMechanism> &&synchronization)
        : synchronization{std::move(synchronization)}
    DrawCommandsQueue::DrawCommandsQueue(std::size_t expectedSize)
    {
        queue.reserve(expectedSize);
    }


@@ 21,16 19,16 @@ namespace service::gui
    {
        cpp_freertos::LockGuard lock{queueMutex};
        queue.push_back(std::move(item));

        synchronization->notify();
    }

    auto DrawCommandsQueue::dequeue() -> QueueItem
    auto DrawCommandsQueue::dequeue() -> std::optional<QueueItem>
    {
        std::optional<QueueItem> item = std::nullopt;
        cpp_freertos::LockGuard lock{queueMutex};
        synchronization->wait(queueMutex, WaitTimeout, [this]() { return !queue.empty(); });

        auto item = std::move(queue.front());
        if (queue.empty()) {
            return item;
        }
        item = std::move(queue.front());
        queue.erase(queue.begin());
        return item;
    }

M module-services/service-gui/DrawCommandsQueue.hpp => module-services/service-gui/DrawCommandsQueue.hpp +5 -8
@@ 1,11 1,10 @@
// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved.
// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved.
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#pragma once

#include "SynchronizationMechanism.hpp"

#include <gui/core/DrawCommand.hpp>
#include <mutex.hpp>

#include <cstdint>
#include <list>


@@ 25,12 24,11 @@ namespace service::gui
        };
        using QueueContainer = std::vector<QueueItem>;

        explicit DrawCommandsQueue(
            std::size_t expectedSize,
            std::unique_ptr<SynchronizationMechanism> &&synchronization = getFreeRtosSynchronizationMechanism());
        explicit DrawCommandsQueue(std::size_t expectedSize);

        void enqueue(QueueItem &&item);
        [[nodiscard]] auto dequeue() -> QueueItem;
        auto stop() -> void;
        [[nodiscard]] auto dequeue() -> std::optional<QueueItem>;
        [[nodiscard]] auto getMaxRefreshModeAndClear() -> ::gui::RefreshModes;
        void clear();
        [[nodiscard]] auto size() const noexcept -> QueueContainer::size_type;


@@ 39,6 37,5 @@ namespace service::gui
        QueueContainer queue;

        mutable cpp_freertos::MutexStandard queueMutex;
        std::unique_ptr<SynchronizationMechanism> synchronization;
    };
} // namespace service::gui

M module-services/service-gui/WorkerGUI.cpp => module-services/service-gui/WorkerGUI.cpp +5 -5
@@ 1,4 1,4 @@
// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved.
// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved.
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#include "WorkerGUI.hpp"


@@ 39,7 39,9 @@ namespace service::gui
        switch (command) {
        case Signal::Render: {
            auto item = guiService->commandsQueue->dequeue();
            render(item.commands, item.refreshMode);
            if (item.has_value()) {
                render(item->commands, item->refreshMode);
            }
            break;
        }
        case Signal::ChangeColorScheme: {


@@ 54,13 56,10 @@ namespace service::gui
    void WorkerGUI::render(DrawCommandsQueue::CommandList &commands, ::gui::RefreshModes refreshMode)
    {
        const auto [contextId, context] = guiService->contextPool->borrowContext(); // Waits for the context.

        renderer.render(context, commands);

#if DEBUG_EINK_REFRESH == 1
        LOG_INFO("Render ContextId: %d\n%s", contextId, context->toAsciiScaled().c_str());
#endif

        onRenderingFinished(contextId, refreshMode);
    }



@@ 74,4 73,5 @@ namespace service::gui
        auto msg = std::make_shared<service::gui::RenderingFinished>(contextId, refreshMode);
        guiService->bus.sendUnicast(std::move(msg), guiService->GetName());
    }

} // namespace service::gui

M module-services/service-gui/tests/test-DrawCommandsQueue.cpp => module-services/service-gui/tests/test-DrawCommandsQueue.cpp +7 -8
@@ 1,10 1,9 @@
// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved.
// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved.
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#include <catch2/catch.hpp>

#include "DrawCommandsQueue.hpp"
#include "MockedSynchronizationMechanism.hpp"

#include <chrono>
#include <thread>


@@ 14,7 13,7 @@ using namespace service::gui;
TEST_CASE("DrawCommandsQueueTests")
{
    constexpr auto ExpectedQueueSize = 4U;
    DrawCommandsQueue queue{ExpectedQueueSize, std::make_unique<MockedSynchronizationMechanism>()};
    DrawCommandsQueue queue{ExpectedQueueSize};

    SECTION("Enqueue")
    {


@@ 28,25 27,25 @@ TEST_CASE("DrawCommandsQueueTests")
        REQUIRE(queue.size() == 1U);

        const auto item = queue.dequeue();
        REQUIRE(item.commands.empty());
        REQUIRE(item.refreshMode == ::gui::RefreshModes::GUI_REFRESH_FAST);
        REQUIRE(item->commands.empty());
        REQUIRE(item->refreshMode == ::gui::RefreshModes::GUI_REFRESH_FAST);
        REQUIRE(queue.size() == 0);
    }

    SECTION("Multi-thread dequeue")
    {
        std::thread thr{[&queue]() {
            std::this_thread::sleep_for(std::chrono::milliseconds{100});
            queue.enqueue(DrawCommandsQueue::QueueItem{});
        }};

        std::this_thread::sleep_for(std::chrono::milliseconds{100});
        const auto item = queue.dequeue();
        if (thr.joinable()) {
            thr.join();
        }

        REQUIRE(item.commands.empty());
        REQUIRE(item.refreshMode == ::gui::RefreshModes::GUI_REFRESH_FAST);
        REQUIRE(item->commands.empty());
        REQUIRE(item->refreshMode == ::gui::RefreshModes::GUI_REFRESH_FAST);
        REQUIRE(queue.size() == 0U);
    }


M module-sys/Service/Worker.cpp => module-sys/Service/Worker.cpp +6 -17
@@ 206,8 206,6 @@ namespace sys
    {
        assert(getState() == State::Initiated);

        runnerTask = xTaskGetCurrentTaskHandle();

        BaseType_t task_error = 0;
        task_error            = xTaskCreate(
            Worker::taskAdapter, name.c_str(), stackDepth / sizeof(StackType_t), this, priority, &taskHandle);


@@ 222,12 220,8 @@ namespace sys

    bool Worker::stop()
    {
        if (runnerTask) {
            assert(xTaskGetCurrentTaskHandle() == runnerTask);
            assert(getState() == State::Running);
            return sendControlMessage(ControlMessage::Stop);
        }
        return true;
        assert(getState() == State::Running);
        return sendControlMessage(ControlMessage::Stop);
    }

    bool Worker::sendControlMessage(ControlMessage message)


@@ 243,7 237,6 @@ namespace sys

    bool Worker::send(uint32_t cmd, uint32_t *data)
    {
        assert(xTaskGetCurrentTaskHandle() == runnerTask);
        assert(getState() == State::Running);

        WorkerCommand workerCommand{cmd, data};


@@ 275,15 268,11 @@ namespace sys

    bool Worker::join(TickType_t timeout)
    {
        if (runnerTask) {
            assert(xTaskGetCurrentTaskHandle() == runnerTask);
            assert(getState() == State::Running || getState() == State::Stopped);

            if (xSemaphoreTake(joinSemaphore, timeout) != pdTRUE) {
                return false;
            }
            while (eTaskGetState(taskHandle) != eDeleted) {}
        assert(getState() == State::Running || getState() == State::Stopped);
        if (xSemaphoreTake(joinSemaphore, timeout) != pdTRUE) {
            return false;
        }
        while (eTaskGetState(taskHandle) != eDeleted) {}
        return true;
    }


M module-sys/Service/include/Service/Worker.hpp => module-sys/Service/include/Service/Worker.hpp +1 -2
@@ 1,4 1,4 @@
// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved.
// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved.
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#pragma once


@@ 108,7 108,6 @@ namespace sys
        static constexpr auto controlQueueNamePrefix      = "wctrl";

        xSemaphoreHandle joinSemaphore = nullptr;
        xTaskHandle runnerTask         = nullptr;
        xSemaphoreHandle stateMutex    = nullptr;
        xTaskHandle taskHandle         = nullptr;


M module-utils/log/Logger.cpp => module-utils/log/Logger.cpp +14 -7
@@ 1,4 1,4 @@
// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved.
// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved.
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#include "Logger.hpp"


@@ 166,19 166,24 @@ namespace Log
    /// @return: < 0 - error occured during log flush
    /// @return:   0 - log flush did not happen
    /// @return:   1 - log flush successflul
    auto Logger::dumpToFile(std::filesystem::path logPath) -> int
    auto Logger::dumpToFile(std::filesystem::path logPath, bool isLoggerRunning) -> int
    {
        std::error_code errorCode;
        auto firstDump = !std::filesystem::exists(logPath, errorCode);

        if (errorCode) {
            LOG_ERROR("Failed to check if file %s exists, error: %d", logPath.c_str(), errorCode.value());
            if (isLoggerRunning) {
                LOG_ERROR("Failed to check if file %s exists, error: %d", logPath.c_str(), errorCode.value());
            }
            return -EIO;
        }

        if (const bool maxSizeExceeded = !firstDump && std::filesystem::file_size(logPath) > maxFileSize;
            maxSizeExceeded) {
            LOG_DEBUG("Max log file size exceeded. Rotating log files...");
            if (isLoggerRunning) {
                LOG_DEBUG("Max log file size exceeded. Rotating log files...");
            }

            {
                LockGuard lock(logFileMutex);
                rotator.rotateFile(logPath);


@@ 209,7 214,9 @@ namespace Log
            }
        }

        LOG_DEBUG("Flush ended with status: %d", status);
        if (isLoggerRunning) {
            LOG_DEBUG("Flush ended with status: %d", status);
        }

        return status;
    }


@@ 225,10 232,10 @@ namespace Log
    auto Logger::flushLogs() -> int
    {
        LOG_INFO("Shutdown dump logs");
        worker->kill();
        worker->close();
        writeLogsTimer.stop();
        buffer.nextBuffer();
        return dumpToFile(purefs::dir::getLogsPath() / LOG_FILE_NAME);
        return dumpToFile(purefs::dir::getLogsPath() / LOG_FILE_NAME, false);
    }

    void Logger::checkBufferState()

M module-utils/log/Logger.hpp => module-utils/log/Logger.hpp +2 -2
@@ 1,4 1,4 @@
// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved.
// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved.
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#pragma once


@@ 46,7 46,7 @@ namespace Log
        auto log(logger_level level, const char *file, int line, const char *function, const char *fmt, va_list args)
            -> int;
        auto logAssert(const char *fmt, va_list args) -> int;
        auto dumpToFile(std::filesystem::path logPath) -> int;
        auto dumpToFile(std::filesystem::path logPath, bool isLoggerRunning = true) -> int;
        auto diagnosticDump() -> int;
        auto flushLogs() -> int;