fcitx / fcitx5-chewing

17 stars 7 forks source link

GUI issue in fcitx5-chewing 5.1.3 with libchewing 0.8.0 rc2 #18

Closed yan12125 closed 6 months ago

yan12125 commented 6 months ago

Symptom

Since 4411a3a813207389ce0931b0a9de981869932866 (part of version 5.1.3), the preedit window is doing something strange during typing. For example, when I type 2 ( if the default keyboard layout is configured), I got repeated bopomofo symbols:

圖片

Note that the issue happens only with the Rust version of libchewing (default since 0.8.0 rc1). Everything works fine with libchewing 0.7.0.

Analysis

With libchewing 0.8.0 rc2, logs of RUST_LOG=debug fcitx5 -D --verbose default=5,chewing=5 shows that the bopomofo symbol appears twice:

D2024-05-02 22:10:14.583860 eim.cpp:562] Text: ㄉ Zuin: ㄉ

On the other hand, with libchewing 0.7.0, the bopomofo symbol appears only once in logs:

D2024-05-02 22:09:49.200383 eim.cpp:562] Text:  Zuin: ㄉ

It seems that in the Rust version of libchewing, all *_static functions share the same buffer. Specifically, when there are data to return, *_static functions use an internal helper to copy desired data to a static global array:

https://github.com/chewing/libchewing/blob/v0.8.0-rc.2/capi/src/io.rs#L1451 https://github.com/chewing/libchewing/blob/v0.8.0-rc.2/capi/src/io.rs#L71

As a result, when two *_static functions are called in a row (https://github.com/fcitx/fcitx5-chewing/blob/5.1.3/src/eim.cpp#L557-L558), both resulting pointers point to the same data.

Test environment

/cc @kanru this issue seems related to a difference in Rust and C implementations. I know little about Rust. Could you confirm whether my analysis is correct or not?

wengxt commented 6 months ago

That would be so silly in api design..

Can you open issue on libchewing instead?

kanru commented 6 months ago

The *_static APIs are silly. They shouldn't be used.

I wrote this in new API doc which I should also add to the info manual.

chewing_buffer_String_static:

The return value is a const pointer to a character string. The pointer is only valid immediately after checking the chewing_buffer_Check condition.

The original C functions also have this comment

Always returns a const char pointer, you have to clone them immediately, if you need.

They have the same downside of C's API like errno or strtok and other functions that use internal static buffers -- they are not thread safe and the internal buffer might be modified after subsequent function calls. libchewing was not thread safe until we moved everything to be allocated per-context but the content of the buffer is still ephemeral.

The difference between 0.7.0 libchewing and 0.8.0 is that Rust version shares a global static buffer while C had a static buffer for each output type, though that was never guaranteed. I can change to use buffer per output, but they should still be copied immediately after output.