<AWT Dev> [16] RFR 8232114: JVM crashed at imjpapi.dll in native code

[Anton Litvinov]

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/
>
> Regards,
> Dmitry
>
>> On 20 Aug 2020, at 15:14, Anton Litvinov <ANTON.LITVINOV at ORACLE.COM 
>> <mailto:ANTON.LITVINOV at 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.bylokhov at oracle.com <mailto:sergey.bylokhov at 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/
>>>>> Regards,
>>>>> Dmitry
>>>>>> On 15 Aug 2020, at 03:05, Sergey Bylokhov 
>>>>>> <sergey.bylokhov at oracle.com 
>>>>>> <mailto:sergey.bylokhov at oracle.com><mailto:sergey.bylokhov at 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.Bylokhov at oracle.com 
>>>>>>>> <mailto:Sergey.Bylokhov at oracle.com><mailto:Sergey.Bylokhov at 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
>>>>>>>>
>>>>>>>> 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.
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/awt-dev/attachments/20200825/bff696e5/attachment-0001.htm>