Skip to content

fix(terminal): prevent race conditions and crash on canvas and scheduler teardown #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MorganCaron merged 1 commit into from
Sep 20, 2026
Merged

fix(terminal): prevent race conditions and crash on canvas and scheduler teardown #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions modules/Terminal/Area.mpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,16 @@ export namespace CppUtils::Terminal
m_viewport{(viewport.getSize().width() == 0 or viewport.getSize().height() == 0) ? Viewport{size} : viewport}
{}

virtual inline ~Area() noexcept override
{
clearWidgetManager();
}

template<std::derived_from<Widget> T>
inline auto addWidget(this auto&& self [[clang::lifetimebound, msvc::lifetimebound]], std::unique_ptr<T> widget) -> T&
{
if (self.m_widget)
self.m_widget->clearWidgetManager();
self.m_widget = std::move(widget);
if (self.hasWidgetManager())
self.m_widget->setWidgetManager(self.getWidgetManager());
Expand All @@ -42,6 +49,12 @@ export namespace CppUtils::Terminal
m_widget->setWidgetManager(widgetManager);
}

inline auto onWidgetManagerClear() noexcept -> void override
{
if (m_widget)
m_widget->clearWidgetManager();
}

public:
[[nodiscard]] inline auto getSize() const noexcept -> Container::Size2<> override
{
Expand Down
12 changes: 9 additions & 3 deletions modules/Terminal/Canvas.mpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import CppUtils.Terminal.CharAttributes;
import CppUtils.Terminal.TextStyle;
import CppUtils.Terminal.TextColor;
import CppUtils.Terminal.BackgroundColor;
import CppUtils.Execution.EventDispatcher;

export namespace CppUtils::Terminal
{
Expand All @@ -32,16 +33,21 @@ export namespace CppUtils::Terminal
m_clearOnClose{clearOnClose},
m_previousBuffer{size}
{
m_widgetManager.eventDispatcher.subscribe<"RequestUpdate">([this] {
m_updateSubscriptionIdentifier = m_widgetManager.eventDispatcher.subscribe<"RequestUpdate">([this] {
print();
});
setWidgetManager(m_widgetManager);
setConsoleOutputUTF8();
enableAnsi();
}

inline ~Canvas() noexcept
inline ~Canvas() noexcept override
{
m_widgetManager.eventDispatcher.unsubscribe(m_updateSubscriptionIdentifier);
m_widgetManager.scheduler.cancelAll();
clearWidgetManager();

auto lock = std::scoped_lock{m_printMutex};
if (m_clearOnClose and not m_firstPrint)
{
const auto terminalSize = getTerminalSize();
Expand All @@ -56,7 +62,6 @@ export namespace CppUtils::Terminal
std::fflush(stdout);
}
m_widget.reset();
clearWidgetManager();
}

inline auto applyDifferences() noexcept -> void
Expand Down Expand Up @@ -225,5 +230,6 @@ export namespace CppUtils::Terminal
bool m_clearOnClose = false;
DynamicAreaBuffer m_previousBuffer;
WidgetManager m_widgetManager;
Execution::EventDispatcher::SubscriptionIdentifier m_updateSubscriptionIdentifier{};
};
}
13 changes: 13 additions & 0 deletions modules/Terminal/Container.mpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,16 @@ export namespace CppUtils::Terminal
addWidget(std::move(widget));
}

inline ~Container() noexcept override
{
clearWidgetManager();
}

template<std::derived_from<Widget> T>
inline auto addWidget(this auto&& self [[clang::lifetimebound, msvc::lifetimebound]], std::unique_ptr<T> widget) -> T&
{
if (self.m_widget)
self.m_widget->clearWidgetManager();
self.m_widget = std::move(widget);
if (self.hasWidgetManager())
self.m_widget->setWidgetManager(self.getWidgetManager());
Expand Down Expand Up @@ -69,6 +76,12 @@ export namespace CppUtils::Terminal
m_widget->setWidgetManager(widgetManager);
}

inline auto onWidgetManagerClear() noexcept -> void override
{
if (m_widget)
m_widget->clearWidgetManager();
}

inline auto onMouseEvent(const Mouse::Event& event) -> bool override
{
if (not m_widget)
Expand Down
11 changes: 11 additions & 0 deletions modules/Terminal/Layout.mpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ export namespace CppUtils::Terminal
m_alignItems{alignItems}
{}

inline ~Layout() noexcept override
{
clearWidgetManager();
}

template<std::derived_from<Element> T>
inline auto addWidget(this auto&& self [[clang::lifetimebound, msvc::lifetimebound]], std::unique_ptr<T> widget) -> T&
{
Expand All @@ -63,6 +68,12 @@ export namespace CppUtils::Terminal
element->setWidgetManager(widgetManager);
}

inline auto onWidgetManagerClear() noexcept -> void override
{
for (auto& element : m_elements)
element->clearWidgetManager();
}

public:
inline auto setDirection(this auto&& self, Direction direction) noexcept -> decltype(auto)
{
Expand Down
10 changes: 10 additions & 0 deletions modules/Terminal/Scrollable.mpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export namespace CppUtils::Terminal
m_fullContentArea{fullContentSize}
{}

inline ~Scrollable() noexcept override
{
clearWidgetManager();
}

template<std::derived_from<Widget> T>
inline auto addWidget(std::unique_ptr<T> widget) -> T&
{
Expand All @@ -49,6 +54,11 @@ export namespace CppUtils::Terminal
m_fullContentArea.setWidgetManager(widgetManager);
}

inline auto onWidgetManagerClear() noexcept -> void override
{
m_fullContentArea.clearWidgetManager();
}

public:
inline auto setScroll(this auto&& self, const Container::Size2<>& position) noexcept -> decltype(auto)
{
Expand Down
2 changes: 2 additions & 0 deletions modules/Terminal/Widget.mpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export namespace CppUtils::Terminal
if (hasWidgetManager())
{
getWidgetManager().scheduler.cancelOwner(this);
onWidgetManagerClear();
m_widgetManagerRef = std::nullopt;
}
}
Expand Down Expand Up @@ -98,6 +99,7 @@ export namespace CppUtils::Terminal

protected:
virtual inline auto onWidgetManagerSet([[maybe_unused]] WidgetManager& widgetManager) noexcept -> void {}
virtual inline auto onWidgetManagerClear() noexcept -> void {}

private:
inline auto requestUpdateImpl(Thread::Scheduler::Clock::duration delay) -> void
Expand Down
20 changes: 13 additions & 7 deletions modules/Thread/Scheduler.mpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export namespace CppUtils::Thread

inline ~Scheduler() noexcept
{
m_threadLoop.requestStop();
m_threadLoop.stop();
cancelAll();
}

Expand Down Expand Up @@ -136,6 +136,7 @@ export namespace CppUtils::Thread
}

tasksToRun.assign(std::ranges::begin(set), std::ranges::end(set));
activeItems.insert(std::ranges::end(activeItems), std::ranges::begin(set), std::ranges::end(set));
processingTasks += std::ranges::size(set);
map.clear();
set.clear();
Expand All @@ -161,6 +162,10 @@ export namespace CppUtils::Thread
m_workCondition.notify_all();
m_finishedCondition.notify_all();
}

for (auto& item : activeItems)
if (item->id == id)
item->cancelled = true;
}

inline auto cancelOwner(const void* owner) -> void
Expand Down Expand Up @@ -205,6 +210,11 @@ export namespace CppUtils::Thread
item->cancelled = true;
map.clear();
set.clear();

m_finishedCondition.wait(accessor.getLockGuard(), [&] {
return std::ranges::empty(activeItems);
});

m_workCondition.notify_all();
m_finishedCondition.notify_all();
}
Expand Down Expand Up @@ -236,6 +246,7 @@ export namespace CppUtils::Thread
if ((*it)->time <= endTime)
{
tasksToRun.push_back(*it);
activeItems.push_back(*it);
map.erase((*it)->id);
it = set.erase(it);
++accessor.value().processingTasks;
Expand All @@ -255,15 +266,10 @@ export namespace CppUtils::Thread
{
for (auto&& item : tasks)
{
{
auto accessor = m_items.access();
accessor.value().activeItems.push_back(item);
}

try
{
m_threadPool.call([this, item] {
auto scope = Execution::ScopeGuard{[this, &item] {
auto scope = Execution::ScopeGuard{[this, item] {
auto accessor = m_items.access();
std::erase(accessor.value().activeItems, item);
m_finishedCondition.notify_all();
Expand Down
Loading