Skip to content
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.

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

Mozc stops working when trying to convert "えもじ" #741

Closed
yukawa opened this issue May 9, 2023 · 2 comments
Closed

Mozc stops working when trying to convert "えもじ" #741

yukawa opened this issue May 9, 2023 · 2 comments

Comments

@yukawa
Copy link
Collaborator

yukawa commented May 9, 2023

Description
Mozc stops working after trying to convert えもじ

Steps to reproduce
Steps to reproduce the behavior:

  1. Build Mozc as debug binaries and install it
  2. Open notepad.exe
  3. Select and enable Mozc.
  4. Type えもじ
  5. Hit the space key

Expected behavior
After the step 5, you can continue using Mozc.

Actual behavior
An error dialog is shown with the following message.

Screenshots
image

Version or commit-id
0ae4263

Environment

  • OS: Windows 11

Investigations

  • Whether this issue happens on both Ibus and other IMF (e.g. Fcitx, uim).
    • N/A
  • Whether this issue happens on other IMEs (e.g. Anthy, SKK).
    • no
  • What applications this issue happens on (e.g. Chromium, gedit).
    • Any app
  • What applications this issue does not happen on (e.g. Chromium, gedit).
    • not-found
  • (optional) What versions or commit-ids this issue did not happen on
    (e.g. Mozc-2.28.4960.100+24.11.oss).
    • no-investigation.
hiroyuki-komatsu pushed a commit that referenced this issue May 25, 2023
Before tackling #741, let's clean up `IPCServer::{request,response}_`, which are used only for propagating `IPC_{REQUEST,RESPONSE}SIZE` to `win32_ipc.cc`.

There must be no observable behavior change.

#codehealth

PiperOrigin-RevId: 535088979
@yukawa
Copy link
Collaborator Author

yukawa commented May 25, 2023

Oops, the commit message of my 6da3e29 did not intend to close this bug. It's just a preparation. The actual fix will come in the next push.

Also, it seems that my 6da3e29 revealed some uncovered bug in unix_ipc.cc according to the test result.

//ipc:ipc_test                                                           FAILED in 25.1s
  /home/runner/.cache/bazel/_bazel_runner/1b5ecbff193b3dab0a33ad145086fbca/execroot/mozc/bazel-out/k8-fastbuild/testlogs/ipc/ipc_test/test.log

I'll file another bug to keep track of it.

hiroyuki-komatsu pushed a commit that referenced this issue May 26, 2023
It turns out that `win32_ipc.cc` may truncate IPC payload if it's bigger than `IPC_RESPONSESIZE`. It seems that Mozc's IPC implementations in other platforms don't have such a limitation, which is why #741 is observed only in Windows.

This CL takes advantage of so-called message-read mode in Windows NamedPipe.

The first step is to call `SetNamedPipeHandleState` API in the IPC client process so that the file handle returned from `CreateFile` can start behaving in the message-read mode.  This step is unnecessary in the IPC server process because `PIPE_READMODE_MESSAGE` is already set in `CreateNamedPipe`.

The next step is to carefully handle `ERROR_MORE_DATA`.  Basically we need to handle two more cases:

 - `ReadFile` returns `FALSE` with `ERROR_MORE_DATA`
 - `ReadFile` returns `FALSE` with `ERROR_IO_PENDING`, and `GetOverlappedResult` returns `FALSE` with `ERROR_MORE_DATA`

In both cases, we can continue reading the remaining data.

With this CL, `win32_ipc.cc` should be able to handle an arbitrary size of payloads as long as the memory allows.

Closes #741

PiperOrigin-RevId: 535319100
@yukawa
Copy link
Collaborator Author

yukawa commented May 26, 2023

9acf467 is the actual fix.

hiroyuki-komatsu pushed a commit that referenced this issue May 26, 2023
This is a follow up for #741 and #747.

From the viewpoint of platform abstraction, it'd make more sense to first verify how much data can be reliably handled in our IPC infrastructure. In that sense, using predictable test dataset with much lower entropy is considered to be not a major concern in `IPCTest`.

With that, this CL simplifies `GenerateInputData` by mostly filling the data with 'x'.

#codehealth

PiperOrigin-RevId: 535503372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant