mirror of
https://github.com/torlando-tech/pyxis.git
synced 2026-09-23 21:24:21 +00:00
fix(lxmf): set send marker in the LVGL lock section (close completion race)
Greptile round on ba5af76 (4/5) correctly rejected the first attempt:
the submitted-text marker was assigned in ChatScreen::on_send_clicked
AFTER the mailbox publish returned, so the main loop could take() +
admit the send and enter clear_composer() while the marker was still
empty — neither clearing the submitted text nor associating the commit
with its submission.
The marker is now recorded by the send callback itself
(UIManager::on_send_message_from_chat) immediately after the mailbox
accept, in the same LVGL lock section as the publish. The click handler
runs on the LVGL task with the LVGL mutex held (LVGLInit.cpp:160-179
wraps the whole lv_task_handler in the recursive mutex), so the marker
is visible to the main loop only after the mailbox entry is — the
take() + admit + clear sequence can never observe an empty marker for
an accepted send. clear_composer() additionally no-ops on an empty
marker, which is the retained-text path for rejected/retry sends.
The contract test is tightened to assert the marker is NOT assigned in
the click handler and IS assigned in the callback, so the race cannot
silently regress.
Verification: 181/181 contracts, tdeck + tdeck-release green.
This commit is contained in:
@@ -768,32 +768,42 @@ void ChatScreen::on_send_clicked(lv_event_t* event) {
|
||||
String message(text);
|
||||
|
||||
if (message.length() > 0 && screen->_send_message_callback) {
|
||||
// Publish to the main loop. On acceptance, record the exact composer
|
||||
// state that was submitted; the composer is cleared only after
|
||||
// persistence and queue admission succeed (clear_composer() from
|
||||
// UIManager::apply_outbound_result) AND only if the composer still
|
||||
// holds that same text. A rejected send (busy router, full queue,
|
||||
// storage error) keeps the text for a normal re-send, and anything
|
||||
// typed into the composer after submission is never wiped by the
|
||||
// later completion commit.
|
||||
if (screen->_send_message_callback(message)) {
|
||||
screen->_pending_submitted_text = message.c_str();
|
||||
}
|
||||
// Publish to the main loop. On acceptance the callback records the
|
||||
// exact composer state that was submitted (inside the callback, so
|
||||
// the marker is set under this same LVGL lock section, BEFORE the
|
||||
// main loop can observe the mailbox entry). The composer is cleared
|
||||
// only after persistence and queue admission succeed AND only when
|
||||
// it still holds that same text. A rejected send keeps the text for
|
||||
// a normal re-send, and anything typed after submission is never
|
||||
// wiped by the later completion commit.
|
||||
screen->_send_message_callback(message);
|
||||
}
|
||||
}
|
||||
|
||||
void ChatScreen::set_pending_submitted_text(const std::string& text) {
|
||||
// Recursive lock: the send callback runs on the LVGL task under the
|
||||
// LVGL lock (on_send_message_from_chat holds it via the click handler's
|
||||
// event context); this keeps the marker consistent with the composer.
|
||||
LVGL_LOCK();
|
||||
_pending_submitted_text = text;
|
||||
}
|
||||
|
||||
void ChatScreen::clear_composer() {
|
||||
// Recursive lock: apply_outbound_result() calls this while already
|
||||
// holding the LVGL lock.
|
||||
LVGL_LOCK();
|
||||
// Only clear if the composer still holds the exact text we submitted.
|
||||
// Persistence + router admission run on the main loop off the LVGL
|
||||
// lock, so the user may have started typing the next message before the
|
||||
// commit lands; wiping an edited composer here would erase fresh input.
|
||||
// An empty _pending_submitted_text means nothing is pending, in which
|
||||
// case a clear is a no-op-safe reset.
|
||||
// Only clear when a submission is pending (non-empty marker) and the
|
||||
// composer still holds exactly the submitted text. Persistence + router
|
||||
// admission run on the main loop off the LVGL lock, so the user may
|
||||
// have started typing the next message before the commit lands; wiping
|
||||
// an edited composer here would erase fresh input. An empty marker
|
||||
// (rejected/retained send, or no submission) clears nothing — the
|
||||
// retained text is kept deliberately for a normal re-send.
|
||||
if (_pending_submitted_text.empty()) {
|
||||
return;
|
||||
}
|
||||
const char* current = lv_textarea_get_text(_text_area);
|
||||
if (current != nullptr && _pending_submitted_text != current) {
|
||||
if (current == nullptr || _pending_submitted_text != current) {
|
||||
return;
|
||||
}
|
||||
_pending_submitted_text.clear();
|
||||
|
||||
@@ -155,10 +155,24 @@ public:
|
||||
|
||||
/**
|
||||
* Set callback for sending messages
|
||||
* @param callback Function to call when send button is pressed
|
||||
* @param callback Function that sends the message; return true when the
|
||||
* send was accepted into the main-loop mailbox. The callback
|
||||
* (UIManager::on_send_message_from_chat) records the submitted
|
||||
* text via set_pending_submitted_text() in the same LVGL lock
|
||||
* section as the publish, so the main loop can never observe
|
||||
* the mailbox entry before the marker is set.
|
||||
*/
|
||||
void set_send_message_callback(SendMessageCallback callback);
|
||||
|
||||
/**
|
||||
* Record the composer text that was just published to the main-loop
|
||||
* send mailbox. Must be called by the send callback in the same LVGL
|
||||
* lock section as the publish so the completion commit (main loop) can
|
||||
* match its clear to the exact submission. See ChatScreen.h field
|
||||
* _pending_submitted_text.
|
||||
*/
|
||||
void set_pending_submitted_text(const std::string& text);
|
||||
|
||||
/**
|
||||
* Set callback for voice call button
|
||||
* @param callback Function to call when call button is pressed
|
||||
@@ -198,9 +212,15 @@ private:
|
||||
std::deque<MessageItem> _messages;
|
||||
|
||||
// Composer text captured when a send was accepted into the main-loop
|
||||
// mailbox. apply_outbound_result() only clears the composer if it still
|
||||
// holds exactly this text, so input typed into the composer while
|
||||
// persistence/admission was in flight is never erased by a later commit.
|
||||
// mailbox. apply_outbound_result() only clears the composer when a
|
||||
// non-empty marker matches the current composer text exactly, so input
|
||||
// typed into the composer while persistence/admission was in flight is
|
||||
// never erased by a later commit. Empty means "no pending submission",
|
||||
// which clears nothing (safe for retry/rejected sends, where the text
|
||||
// is retained deliberately). The marker is assigned by
|
||||
// UIManager::on_send_message_from_chat() in the same LVGL lock section
|
||||
// as the mailbox publish, so the main loop can never observe the
|
||||
// mailbox entry before the marker is set.
|
||||
std::string _pending_submitted_text;
|
||||
|
||||
// Map message hash to bubble row for targeted updates
|
||||
|
||||
@@ -1431,7 +1431,19 @@ void UIManager::on_back_to_conversation_list() {
|
||||
}
|
||||
|
||||
bool UIManager::on_send_message_from_chat(const String& content) {
|
||||
return send_message(_current_peer_hash, content);
|
||||
const bool accepted = send_message(_current_peer_hash, content);
|
||||
if (accepted && _chat_screen) {
|
||||
// Race-critical: set the submitted-text marker in the SAME LVGL lock
|
||||
// section as the mailbox publish above. If the marker were assigned
|
||||
// later (on the ChatScreen side, after this callback returned), the
|
||||
// main loop could take() + admit the send and run clear_composer()
|
||||
// while the marker was still empty, neither clearing the submitted
|
||||
// text nor matching the later commit to its submission. This handler
|
||||
// runs on the LVGL task (click event, lock held), so the marker is
|
||||
// visible to the main loop only after the mailbox entry is.
|
||||
_chat_screen->set_pending_submitted_text(content.c_str());
|
||||
}
|
||||
return accepted;
|
||||
}
|
||||
|
||||
void UIManager::on_call_from_chat() {
|
||||
|
||||
@@ -51,29 +51,47 @@ def test_clear_composer_is_generation_checked():
|
||||
"void ChatScreen::clear_composer()",
|
||||
"lv_group_focus_obj(_text_area);",
|
||||
)
|
||||
# The clear must compare the current composer text against the
|
||||
# captured submitted text and bail when they differ.
|
||||
assert "_pending_submitted_text" in body, (
|
||||
"clear_composer() no longer consults the submitted-text generation"
|
||||
# The clear must bail when no submission is pending (empty marker) and
|
||||
# compare the current composer text against the captured submitted text.
|
||||
assert "_pending_submitted_text.empty()" in body, (
|
||||
"clear_composer() must no-op when no submission is pending"
|
||||
)
|
||||
assert "_pending_submitted_text" in body
|
||||
assert "lv_textarea_get_text(_text_area)" in body
|
||||
assert "return;" in body, (
|
||||
"clear_composer() must skip the clear when the composer was edited"
|
||||
)
|
||||
|
||||
|
||||
def test_send_click_records_submitted_text():
|
||||
def test_send_path_records_submitted_text_in_lock_section():
|
||||
source = chat_source()
|
||||
ui_source = (ROOT / "lib/tdeck_ui/UI/LXMF/UIManager.cpp").read_text()
|
||||
# The ChatScreen click handler must not assign the marker itself — the
|
||||
# marker has to be set inside the callback (UIManager), in the same LVGL
|
||||
# lock section as the mailbox publish, or the main loop can observe the
|
||||
# mailbox entry before the marker exists (Greptile P1: completion race).
|
||||
body = function_body(
|
||||
source,
|
||||
"void ChatScreen::on_send_clicked(",
|
||||
"void ChatScreen::clear_composer()",
|
||||
"void ChatScreen::set_pending_submitted_text(",
|
||||
)
|
||||
assert "_pending_submitted_text" in body, (
|
||||
"on_send_clicked() must capture the submitted composer text on "
|
||||
"acceptance so the later completion can identify it"
|
||||
assert "_pending_submitted_text" not in body, (
|
||||
"on_send_clicked() must not assign the marker outside the callback "
|
||||
"lock section (race with the main-loop completion)"
|
||||
)
|
||||
assert "_send_message_callback(message)" in body
|
||||
# The callback-side handler sets the marker right after acceptance,
|
||||
# under the same LVGL lock the click handler already holds.
|
||||
ui_body = function_body(
|
||||
ui_source,
|
||||
"bool UIManager::on_send_message_from_chat(const String& content)",
|
||||
"void UIManager::on_call_from_chat()",
|
||||
)
|
||||
assert "set_pending_submitted_text" in ui_body, (
|
||||
"on_send_message_from_chat must record the submitted text in the "
|
||||
"same LVGL lock section as the mailbox publish"
|
||||
)
|
||||
assert "send_message(_current_peer_hash, content)" in ui_body
|
||||
|
||||
|
||||
def test_same_peer_reopen_checks_live_message_count():
|
||||
|
||||
Reference in New Issue
Block a user