Thank you, Anton! Sergey, Any remarks regarding the updated fix? http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/ <http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/>
Thanks in advance, Dmitry > On 25 Aug 2020, at 00:35, Anton Litvinov <anton.litvi...@oracle.com> wrote: > > Hi Dmitry, > > The fix looks good. Thank you for taking into account my suggestions. > > Thank you, > Anton > > On 24/08/2020 10:10, Dmitry Markov wrote: >> 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 >>> <mailto: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. >>>> >>> >> >
