Defer setting an element's text if it has children to fix race condition crash, bump version to 1.2.0-rc3

This commit is contained in:
Mr-Wiseguy 2025-05-02 06:33:44 -04:00
parent a03a0faaeb
commit 88c47bcc4e
4 changed files with 53 additions and 4 deletions

View file

@ -48,7 +48,7 @@
#include "../../lib/rt64/src/contrib/stb/stb_image.h"
const std::string version_string = "1.2.0-rc2";
const std::string version_string = "1.2.0-rc3";
template<typename... Ts>
void exit_error(const char* str, Ts ...args) {

View file

@ -36,7 +36,8 @@ namespace recompui {
Element root_element;
Element* autofocus_element = nullptr;
std::vector<Element*> loose_elements;
std::unordered_set<ResourceId> to_update;
std::unordered_set<ResourceId> to_update;
std::vector<std::pair<ResourceId, std::string>> to_set_text;
bool captures_input = true;
bool captures_mouse = true;
Context(Rml::ElementDocument* document) : document(document), root_element(document) {}
@ -67,6 +68,8 @@ enum class ContextErrorType {
AddResourceToWrongContext,
UpdateElementWithoutContext,
UpdateElementInWrongContext,
SetTextElementWithoutContext,
SetTextElementInWrongContext,
GetResourceWithoutOpen,
GetResourceFailed,
DestroyResourceWithoutOpen,
@ -119,6 +122,12 @@ void context_error(recompui::ContextId id, ContextErrorType type) {
case ContextErrorType::UpdateElementInWrongContext:
error_message = "Attempted to update a UI element in a different UI context than the one that's open";
break;
case ContextErrorType::SetTextElementWithoutContext:
error_message = "Attempted to set the text of a UI element with no open UI context";
break;
case ContextErrorType::SetTextElementInWrongContext:
error_message = "Attempted to set the text of a UI element in a different UI context than the one that's open";
break;
case ContextErrorType::GetResourceWithoutOpen:
error_message = "Attempted to get a UI resource with no open UI context";
break;
@ -407,6 +416,22 @@ void recompui::ContextId::process_updates() {
static_cast<Element*>(cur_resource->get())->handle_event(update_event);
}
std::vector<std::pair<ResourceId, std::string>> to_set_text = std::move(opened_context->to_set_text);
// Delete the Rml elements that are pending deletion.
for (auto cur_text_update : to_set_text) {
resource_slotmap::key cur_key{ cur_text_update.first.slot_id };
std::unique_ptr<Style>* cur_resource = opened_context->resources.get(cur_key);
// Make sure the resource exists before setting its text, as it may have been deleted.
if (cur_resource == nullptr) {
continue;
}
// Perform the text update.
static_cast<Element*>(cur_resource->get())->base->SetInnerRML(cur_text_update.second);
}
}
bool recompui::ContextId::captures_input() {
@ -514,6 +539,20 @@ void recompui::ContextId::queue_element_update(ResourceId element) {
opened_context->to_update.emplace(element);
}
void recompui::ContextId::queue_set_text(ResourceId resource, std::string&& text) {
// Ensure a context is currently opened by this thread.
if (opened_context_id == ContextId::null()) {
context_error(*this, ContextErrorType::SetTextElementWithoutContext);
}
// Check that the context that was specified is the same one that's currently open.
if (*this != opened_context_id) {
context_error(*this, ContextErrorType::SetTextElementInWrongContext);
}
opened_context->to_set_text.emplace_back(std::make_pair(resource, std::move(text)));
}
recompui::Style* recompui::ContextId::create_style() {
return add_resource_impl(std::make_unique<Style>());
}

View file

@ -31,6 +31,7 @@ namespace recompui {
void add_loose_element(Element* element);
void queue_element_update(ResourceId element);
void queue_set_text(ResourceId resource, std::string&& text);
Style* create_style();

View file

@ -377,8 +377,17 @@ std::string escape_rml(std::string_view string)
void Element::set_text(std::string_view text) {
if (can_set_text) {
// Escape the string into Rml to prevent element injection.
base->SetInnerRML(escape_rml(text));
if (base->GetNumChildren() != 0) {
// Queue the text update. If it's applied immediately, it might happen
// while the document is being updated or rendered. This can cause a crash
// due to the child elements being deleted while the document is being updated.
// Queueing them defers it to the update thread, which prevents that issue.
// Escape the string into Rml to prevent element injection.
get_current_context().queue_set_text(resource_id, escape_rml(text));
}
else {
base->SetInnerRML(escape_rml(text));
}
}
else {
assert(false && "Attempted to set text of an element that cannot have its text set.");