Fix the candidate window freezing the input method at 100% CPU
The animation frame timer was re-armed with setTime(), setOneShot() and then setEnabled(true). In the systemd event loop backend setEnabled(true) means SD_EVENT_ON, a repeating source, and setOneShot() had just set SD_EVENT_ONESHOT. The re-armed timer was therefore repeating at a time that had already passed, and the event loop fired it again on every iteration: the main thread of Fcitx5 sat at 100% CPU and typing froze. The first animation was not affected, the timer is only re-armed from the second one on, which is why it showed up as a sudden freeze after typing for a while. setOneShot() alone is correct. tests/timer_rearm.cpp arms a timer twice the way the window does: with the old calls it fires 198725 times in 200 ms, with the fix exactly twice. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
8d14454bb9
commit
bf3c88acc8
2 files changed
+81
-1
No files matched your search
+4
-1
@@ -1178,9 +1178,12 @@ void InputWindow::scheduleFrame() {
|
|||||||
});
|
});
|
||||||
frameTimer_->setOneShot();
|
frameTimer_->setOneShot();
|
||||||
} else {
|
} else {
|
||||||
|
// setOneShot() arms the source for one more shot. Do not call
|
||||||
|
// setEnabled(true) after it, that switches back to a repeating source
|
||||||
|
// whose time has already passed, which the event loop then fires on
|
||||||
|
// every iteration (100% CPU). See tests/timer_rearm.cpp.
|
||||||
frameTimer_->setTime(nextFrame);
|
frameTimer_->setTime(nextFrame);
|
||||||
frameTimer_->setOneShot();
|
frameTimer_->setOneShot();
|
||||||
frameTimer_->setEnabled(true);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
// Regression test for the animation frame timer.
|
||||||
|
//
|
||||||
|
// Re-arming a Fcitx5 time event for one more shot has to be setTime() followed by
|
||||||
|
// setOneShot(). Calling setEnabled(true) afterwards switches the source back to
|
||||||
|
// SD_EVENT_ON, and with a time that has already passed the event loop fires it
|
||||||
|
// again on every iteration, which keeps the main thread at 100% CPU.
|
||||||
|
//
|
||||||
|
// g++ -std=c++20 tests/timer_rearm.cpp -o /tmp/timer_rearm \
|
||||||
|
// $(pkg-config --cflags --libs Fcitx5Utils) && /tmp/timer_rearm
|
||||||
|
#include <fcitx-utils/event.h>
|
||||||
|
#include <fcitx-utils/log.h>
|
||||||
|
|
||||||
|
#include <chrono>
|
||||||
|
#include <cstdio>
|
||||||
|
#include <memory>
|
||||||
|
|
||||||
|
using Clock = std::chrono::steady_clock;
|
||||||
|
|
||||||
|
static uint64_t usecIn(std::chrono::milliseconds ms) {
|
||||||
|
return std::chrono::duration_cast<std::chrono::microseconds>(
|
||||||
|
(Clock::now() + ms).time_since_epoch())
|
||||||
|
.count();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Arms a timer twice, like two animations in a row, and counts how often the
|
||||||
|
// callback runs during 200 ms.
|
||||||
|
static int run(bool buggy) {
|
||||||
|
fcitx::EventLoop loop;
|
||||||
|
int fired = 0;
|
||||||
|
std::unique_ptr<fcitx::EventSourceTime> timer;
|
||||||
|
auto arm = [&] {
|
||||||
|
if (!timer) {
|
||||||
|
timer = loop.addTimeEvent(CLOCK_MONOTONIC, usecIn(std::chrono::milliseconds(10)), 1000,
|
||||||
|
[&](fcitx::EventSourceTime *, uint64_t) {
|
||||||
|
fired++;
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
timer->setOneShot();
|
||||||
|
} else {
|
||||||
|
timer->setTime(usecIn(std::chrono::milliseconds(10)));
|
||||||
|
timer->setOneShot();
|
||||||
|
if (buggy) {
|
||||||
|
timer->setEnabled(true); // what scheduleFrame used to do
|
||||||
|
}
|
||||||
|
}
|
||||||
|
};
|
||||||
|
auto stop = loop.addTimeEvent(CLOCK_MONOTONIC, usecIn(std::chrono::milliseconds(200)), 1000,
|
||||||
|
[&](fcitx::EventSourceTime *, uint64_t) {
|
||||||
|
loop.exit();
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
auto second = loop.addTimeEvent(CLOCK_MONOTONIC, usecIn(std::chrono::milliseconds(60)), 1000,
|
||||||
|
[&](fcitx::EventSourceTime *, uint64_t) {
|
||||||
|
arm(); // the second animation
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
arm(); // the first animation
|
||||||
|
loop.exec();
|
||||||
|
return fired;
|
||||||
|
}
|
||||||
|
|
||||||
|
int main() {
|
||||||
|
const int buggy = run(true);
|
||||||
|
const int fixed = run(false);
|
||||||
|
std::printf("old pattern (setTime, setOneShot, setEnabled): %d callbacks in 200 ms\n", buggy);
|
||||||
|
std::printf("new pattern (setTime, setOneShot): %d callbacks in 200 ms\n", fixed);
|
||||||
|
// Two animations, so two shots are right.
|
||||||
|
if (fixed != 2) {
|
||||||
|
std::printf("FAIL: expected exactly 2 callbacks\n");
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
if (buggy <= 2) {
|
||||||
|
std::printf("note: the old pattern did not misbehave on this build\n");
|
||||||
|
}
|
||||||
|
std::printf("ok\n");
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
Reference in new issue
Block a user