Hi Anton, Thank you for review. I have updated the fix based on your suggestions: - Modified the function InvokeInputMethodFunction() to take into account the result of PostMessage() call - Replaced InvokeInputMethodFunction() with SendMessage() for WM_AWT_HANDLE_NATIVE_IME_EVENT and WM_AWT_ACTIVATEKEYBOARDLAYOUT. I didn’t update the code related to WM_AWT_OPENCANDIDATEWINDOW message because we can call IME API during its processing, (e.g. AwtToolkit::WndProc() -> AwtComponent::OpenCandidateWindow() -> AwtComponent::SetCandidateWindow() -> Imm*).
The new web rev is located at http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/ <http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/> Regards, Dmitry > On 20 Aug 2020, at 15:14, Anton Litvinov <anton.litvi...@oracle.com> wrote: > > Hi Dmitry, > > The fix looks well, but I have next a few comments relatively to it: > > 1) In the new function "AwtToolkit::InvokeInputMethodFunction(UINT, WPARAM, > LPARAM)", which you created, the call "PostMessage(msg, wParam, lParam);" may > fail and return "0", so in that case the next call > "::WaitForSingleObject(m_inputMethodWaitEvent, INFINITE);" may become blocked > forever. I suggest to handle this possible failure, for example in the next > way: > > if (PostMessage(msg, wParam, lParam)) { > ::WaitForSingleObject(m_inputMethodWaitEvent, INFINITE); > return m_inputMethodData; > } > return 0; > > 2) In "AwtToolkit::WndProc" function in the code handling the messages: > "WM_AWT_HANDLE_NATIVE_IME_EVENT", "WM_AWT_ACTIVATEKEYBOARDLAYOUT", > "WM_AWT_OPENCANDIDATEWINDOW" I do not find calls to "Imm*" system functions. > Therefore maybe there is no need to send these messages through "PostMessage"? > > Thank you, > Anton > > On 18/08/2020 06:46, Dmitry Markov wrote: >> Thank you, Sergey! >> >> Looking for one more "+1”. Any volunteers? >> >> Regards, >> Dmitry >>> On 17 Aug 2020, at 21:06, Sergey Bylokhov <sergey.bylok...@oracle.com >>> <mailto:sergey.bylok...@oracle.com>> wrote: >>> >>> Looks fine. >>> >>> On 17.08.2020 02:32, Dmitry Markov wrote: >>>> Hi Sergey, >>>> I have added that information to InvokeInputMethodFunction(). Please find >>>> the new webrev here: >>>> http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/ >>>> <http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/> >>>> Regards, >>>> Dmitry >>>>> On 15 Aug 2020, at 03:05, Sergey Bylokhov <sergey.bylok...@oracle.com >>>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com >>>>> <mailto:sergey.bylok...@oracle.com>>> wrote: >>>>> >>>>> On 12.08.2020 05:09, Dmitry Markov wrote: >>>>>> TranslateMessage() does not invoke PeekMessage(). In our case >>>>>> TranslateMessage() is called by AWT. IME functionality may call >>>>>> PeekMessage() during TranslateMessage() execution. However that >>>>>> PeekMessage() call is intended for processing non-queued messages, >>>>>> (i.e. the messages send via SendMessage() call). >>>>>> I contacted Microsoft regarding this problem and one of their >>>>>> suggestions was to use PostMessage() instead of SendMessage() for IME >>>>>> messages to avoid IME internal data corruption and the crash. >>>>>> The proposed fix was tested by the stress test for several weeks and no >>>>>> issues were observed. So I feel quite confident that it eliminates the >>>>>> issue. >>>>>> There is no exact message which triggers the crash. Usually the crash is >>>>>> caused by one of the following messages: WM_AWT_ASSOCIATECONTEXT or >>>>>> WM_AWT_SETOPENSTATUS but several times I observed that it was triggered >>>>>> by WM_AWT_DESTROYCONTEXT or WM_AWT_CREATECONTEXT. It looks like almost >>>>>> every IME-related message may cause the crash. I think SendMessage() >>>>>> call should be substituted by PostMessage() for all IME messages. >>>>> >>>>> Ok, then please add this(or similar) information to the new method >>>>> "InvokeInputMethodFunction", >>>>> otherwise it could be removed in the future/replaced back to the >>>>> sendMessage. >>>>> >>>>>> Regards, >>>>>> Dmitry >>>>>>> On 12 Aug 2020, at 06:16, Sergey Bylokhov <sergey.bylok...@oracle.com >>>>>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com >>>>>>> <mailto:sergey.bylok...@oracle.com>>> wrote: >>>>>>> >>>>>>> Hi, Dmitry. >>>>>>> >>>>>>> On 11.08.2020 01:07, Dmitry Markov wrote: >>>>>>>> Problem description: >>>>>>>> The root cause of the crash is the lack of synchronisation in >>>>>>>> imjpapi.dll. In particular when IME messages are processed in the >>>>>>>> message loop and another message triggered through a SendMessage() >>>>>>>> call, this clears the buffer context so on further processing the >>>>>>>> message loop in IME context will point to invalid memory buffer. >>>>>>>> Microsoft article devoted to this issue: >>>>>>>> https://docs.microsoft.com/en-us/troubleshoot/windows/win32/ime-crash-processing-cross-thread-sent-message >>>>>>>> >>>>>>>> <https://docs.microsoft.com/en-us/troubleshoot/windows/win32/ime-crash-processing-cross-thread-sent-message> >>>>>>> >>>>>>> The documentation above also states that PeekMessage, called by the >>>>>>> TranslateMessage when the IME is ON, can proceed the posted messages as >>>>>>> well if that true then the current fix does not help. >>>>>>> >>>>>>>> Fix: >>>>>>>> Replace SendMessage() with PostMessage() for IME messages and >>>>>>>> implement event based mechanism to notify the sender that the message >>>>>>>> processing is completed. >>>>>>> >>>>>>> What exact message broke the IME, the "WM_AWT_DESTROYCONTEXT"? >>>>>>> >>>>>>>> Testing: >>>>>>>> mach5 client tests (jtreg headful, jck, etc.) are green. >>>>>>>> Regards, >>>>>>>> Dmitry >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Best regards, Sergey. >>>>> >>>>> >>>>> -- >>>>> Best regards, Sergey. >>> >>> >>> -- >>> Best regards, Sergey. >> >
