search

pTykvin / javachromiumembedded

Automatically exported from code.google.com/p/javachromiumembedded
0 stars 0 forks source link

Windows: deadlock if you drag over some kind of data over the webbrowser widget in windowed mode #116

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
What steps will reproduce the problem?
1. Checkout r101
2. Apply patch file "2014-09-15_deadlock_on_drag_over_test_application.patch"
3. Build and run tests.simple.MainFrame in windowed mode
4. Choose an URL out of the list on the left side.
5. Drag the URL with the mouse and move the mouse over the embedded webbrowser 
widget.

What is the expected output? What do you see instead?
Dropping the drag-item onto the browser widget should load the dropped URL. 
Instead the whole applications runs into a deadlock if you touch the browser ui 
with the drag-item on your mouse cursor.

What version of the product are you using? On what operating system?
JCEF Version: r101
CEF Version: 3.1916.1781
Java Version: 1.7u65
on Windows 7.x64

Please provide any additional information below.
The reason for the deadlock is that the CEF message loop has to run on the 
AWT-EventQueue thread. That brings us in the following situation:
a) Starting your drag will create a java drag-item (Transferable)
b) If this drag-item touches the CefBrowser, a "drag-over-event" is fired which 
will be handled in DoMessageLoopWork()
c) DoMessageLoopWork() is called on the AWT-EventQueue thread and the 
"drag-over-event" is handled
d) CEF/Chromium wants to get informations about the drag-item and asks the OS.
e) The OS knows that the drag came from the Java application and asks that 
application
f) The Java application wants the get the drag informations, but that is 
handled in the AWT-EventQueue thread.
g) But we're still waiting in the AWT-EventQueue thread for the drag data and 
therefore java can't step into the thread a second time...
... Ends up in a classic deadlock.

How to solve that deadlock?
-----------------------
It's not quite easy to solve that deadlock, if the handling of the 
CefMessageLoop stays in the AWT-EventQueue thread. But moving it to another 
thread causes other significant problems (e.g. initialize, shutdown, create 
browser, etc. have to be moved to the other thread as well and this will cause 
serious problems with the Java-Swing architecture).

The solution is to enable the multi threaded message loop on windows 
(settings.multi_threaded_message_loop = true) which moves the message handling 
to a new thread but doesn't break the Java-Swing architecture.

The attached patch file 
"2014-09-15_deadlock_on_drag_over_test_application_possible_fix.patch" uses 
this approach, BUT it ends up with a "Check failed: CalledOnValidThread()" 
assertion if you shutdown the application. (btw: initialize and shutdown are 
called on the AWT-EventQueue)

Offscreen rendering disabled
Using:
JCEF Version = 3.1916.1781.101
CEF Version = 3.1916.1781
Chromium Version = 35.0.1916.138
CefApp: INITIALIZING
initialize on Thread[AWT-EventQueue-0,6,main] with library path 
C:\build\jcef\src\out\win64\Debug
Added scheme search://
Added scheme client://
CefApp: INITIALIZED
CefApp: SHUTTING_DOWN
  shutdown on Thread[AWT-EventQueue-0,6,main]
All clients are shut down waiting 5secs.
[0915/074624:FATAL:notification_registrar.cc(76)] Check failed: 
CalledOnValidThread(). 
Backtrace:
    cef_time_to_timet [0x00000001805CE5D6+2555670]
    cef_time_to_timet [0x00000001804F1C6D+1652141]
    SetCrashKeyValueImpl [0x00000001806A3C66+64118]
    SetCrashKeyValueImpl [0x00000001806A2F78+60808]
    SetCrashKeyValueImpl [0x0000000180A91636+4183110]
    SetCrashKeyValueImpl [0x0000000180A91F3C+4185420]
    SetCrashKeyValueImpl [0x0000000180A9251F+4186927]
    SetCrashKeyValueImpl [0x0000000180A933E2+4190706]
    cef_time_to_timet [0x00000001805B9B57+2471063]
    cef_time_to_timet [0x00000001805B96D5+2469909]
    cef_time_to_timet [0x00000001805B9B19+2471001]
    cef_time_to_timet [0x0000000180444A2D+942957]
    cef_time_to_timet [0x00000001805B9806+2470214]
    cef_time_to_timet [0x00000001805B939D+2469085]
    SetCrashKeyValueImpl [0x000000018531051C+80200492]
    SetCrashKeyValueImpl [0x000000018531042E+80200254]
    SetCrashKeyValueImpl [0x00000001853134E2+80212722]
    SetCrashKeyValueImpl [0x0000000185313465+80212597]
    SetCrashKeyValueImpl [0x0000000185312F8A+80211354]
    cef_string_wide_to_utf8 [0x00000001802C4C2A+222250]
    cef_string_wide_to_utf8 [0x00000001802C6035+227381]
    cef_string_wide_to_utf8 [0x00000001802C4AEC+221932]
    cef_shutdown [0x00000001801F98F3+35]
    CefShutdown [0x000007FEFB114890+32] (c:\build\jcef\src\third_party\cef\win64\libcef_dll\wrapper\libcef_dll_wrapper.cc:171)
    Java_org_cef_CefApp_N_1Shutdown [0x000007FEFB0B979D+45] (c:\build\jcef\src\native\cefapp.cpp:113)
    (No symbol) [0x0000000002139AA9]

@Marshall: So I guess there is something I've missed to add but I don't know 
exactly what. Do you have a hint what I've missed?

Regards,
Kai

Original issue reported on code.google.com by k...@censhare.de on 15 Sep 2014 at 6:18

Attachments:

GoogleCodeExporter commented 8 years ago 
What is the current relationship between the |context| Thread created in 
CefApp.java and the AWT-EventQueue thread used for drag&drop? Will they always 
be the same thread? Do they need to be the same thread? Is there some way that 
we could force them to be different threads?

If we can't control which threads Java is using then I think it's OK to proceed 
with the multi-threaded-message-loop approach. The notification_registrar 
thread check failure is 
https://code.google.com/p/chromiumembedded/issues/detail?id=755. It's a debug 
assertion so shouldn't cause problems with a release build of libcef.

Original comment by magreenb...@gmail.com on 18 Sep 2014 at 3:36

GoogleCodeExporter commented 8 years ago 
@#1: The notification_registrar thread check failure is now fixed in CEF.

Original comment by magreenb...@gmail.com on 18 Sep 2014 at 8:41

GoogleCodeExporter commented 8 years ago 
@#1:

The main job of the context thread is to call the native CefDoMessageLoopWork() 
approx. every 33ms and to keep an eye on shutdown requests. Due the 
initialization of CEF is done on the AWT-EventQueue thread, we have to run the 
message loop on the EventQueue as well.

For that reason an implementation of the interface Runnable is created and 
passed to the AWT-EventQueue by calling "SwingUtilities.invokeLater(...)" which 
in fact doesn't block the "context" thread. If the runnable is triggered on the 
AWT-EventQueue thread, it calls N_DoMessageLoopWork() and returns. After that 
the next event is handled by the EventQueue thread.

If you drag an item within the same application and drag over (touching the 
browser UI is enough) the CEF based UI a native "DRAG START" Event will be 
fired to the native CEF (chromium) implementation. This event will be handled 
within the CEF message queue, which means in fact that it will be handled 
within the AWT-EventQueue (the next time N_DoMessageLoopWork() is called).

CEF / chromium wants to collect some data about the drag item and therefore it 
"asks" the OS about it. The OS itself asks the application the drag came from 
and that's in fact the Java code running in the JVM.

Now my guessing starts:
Th OS sends and WindowsEvent to the Java code and I think the handling of this 
WindowsEvents is running in the AWT-EventQueue.
So for that reason the calling OS-thread waits for a reply from the 
AWT-EventQueue. But this reply will never be delivered because 
N_DoMessageLoopWork() is waiting for a reply from the OS-thread and in fact the 
N_DoMessageLoopWork() is currently executed on the AWT-EventQueue....Deadlock!

Changing the behavior of the JVM/JRE isn't possible. So we can' say that the 
Event for "give me the drag data" has to be performed on another thread because 
of Swing's architecture.

So I don't see any other solution than proceeding with the 
multi-threaded-message-loop approach.

@#2: Thanks for your fix. I'll try CEF 3.1916.1839 within the next days.

Regards,
Kai

Original comment by k...@censhare.de on 19 Sep 2014 at 5:45

GoogleCodeExporter commented 8 years ago 
@#3: Will the thread deadlock be an issue on OS X? If not, what do you think 
explains the difference on that platform?

Do you have a final patch for the multi-threaded-message-loop usage on Windows?

Original comment by magreenb...@gmail.com on 29 Sep 2014 at 4:50

GoogleCodeExporter commented 8 years ago 
@#4: The deadlock isn't an issue on OS X because we don't run the CEF message 
loop within the AWT Dispatcher thread. It is handled within the "MainThread" of 
the application by using performSelectorOnMainThread().
With the patch the windows and mac implementations are somewhat more similar in 
the meaning of how the message loop is handled.

Attached you'll find a final patch for this issue. There some additional 
changes compared to the first patch.

Regards,
Kai

Original comment by k...@censhare.de on 2 Oct 2014 at 11:42

Attachments:

GoogleCodeExporter commented 8 years ago 
Here's a slightly different test case.

1. Open tests.details.MainFrame
2. Go to this URL: 
http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_input_type_file
3. Click on "Choose file" on right pane
4. A file picker comes up. Make sure an image file e.g. a png image is visible 
in the list of files displayed in the picker.
5. From within the file picker's list, drag the image file from picker to 
browser's window.
6. Freeze similar to discussed in this defect.

In this case, there's no dragged item from Swing.

Original comment by christop...@gmail.com on 3 Oct 2014 at 8:26

GoogleCodeExporter commented 8 years ago 
Verified that slightly different test case is also fixed with the patch 
discussed here

Original comment by christop...@gmail.com on 6 Oct 2014 at 5:26

GoogleCodeExporter commented 8 years ago 
@#5: Thanks, added in revision 106 with minor changes:
- Use anonymous namespace instead of static functions.
- Rename critical_wait_unix.cpp to critical_wait_posix.cpp for consistency.

Original comment by magreenb...@gmail.com on 24 Oct 2014 at 4:02