Author: toshok
Date: 2006-11-16 00:45:58 -0500 (Thu, 16 Nov 2006)
New Revision: 67972
Modified:
trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/ChangeLog
trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/Hwnd.cs
trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/XEventQueue.cs
trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/XplatUIX11.cs
Log:
2006-11-16 Chris Toshok <[EMAIL PROTECTED]>
* XplatUIX11.cs: nasty, nasty, nasty changes required because of
the broken multithreaded event handling we have in here. File
this entry under "Why we should move to the new X11 backend".
Any thread can make it into UpdateMessageQueue, which gets events
from the X socket - some of which could belong to hwnds being
managed by a different thread. We can also have multiple threads
in UpdateMessageQueue at the same time, with each one reading from
the X socket. This leads to many problems, with the following
solutions:
We can't use hwnd.Queue.Enqueue anywhere in here and must use
EnqueueLocked.
The MotionNotify compression we do can't work across threads
(without locking the entire queue, perhaps) since we call
hwnd.Queue.Peek, so we just punt and don't compress motion events
unless the owning thread is the one which got the X event.
ConfigureNotify is another fun one, since it modifies the hwnd's
bounds and then enqueues the event. We add a lock to Hwnd which
is held when setting configure_pending to true (and enqueuing the
event).
There is a race wrt the wake socket. we need to make sure that
only 1 thread is waiting on that socket, or else a thread could
sleep waiting for data that never comes. It's difficult (but not
impossible) to make happen, because it seems to require something
like the following:
1. Thread 1 polls on wake_receive
2. poll returns saying there's data to be read on
wake_receive.
3. Thread 2 polls on wake_receive and immediately returns
saying there's data to be read.
4. Thread 2 reads the wakeup byte from wake_receive
5. Thread 1 attempts to read the wakeup byte from
wake_receive.
6. Thread 2 exits (due to a form closing, perhaps).
7. Thread 1 blocks forever.
Fun, eh?
Fixing the Expose handling isn't done yet, and the races inherent
in that piece of code are responsible for the drawing mistakes you
see when generating expose events in a MT app (like NPlot). This
one is the likely to be the hardest to bandaid, and it doesn't
appear to cause anything but drawing problems. The other issues
caused apps to exit or hang.
* XEventQueue.cs: output some spew when Dequeue/Enqueue/Peek are
called from a different thread than the one that should be calling
these functions.
* Hwnd.cs: add some locks to be used by the XplatUIX11 code.
Modified: trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/ChangeLog
===================================================================
--- trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/ChangeLog
2006-11-16 01:54:29 UTC (rev 67971)
+++ trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/ChangeLog
2006-11-16 05:45:58 UTC (rev 67972)
@@ -1,3 +1,67 @@
+2006-11-16 Chris Toshok <[EMAIL PROTECTED]>
+
+ * XplatUIX11.cs: nasty, nasty, nasty changes required because of
+ the broken multithreaded event handling we have in here. File
+ this entry under "Why we should move to the new X11 backend".
+
+ Any thread can make it into UpdateMessageQueue, which gets events
+ from the X socket - some of which could belong to hwnds being
+ managed by a different thread. We can also have multiple threads
+ in UpdateMessageQueue at the same time, with each one reading from
+ the X socket. This leads to many problems, with the following
+ solutions:
+
+ We can't use hwnd.Queue.Enqueue anywhere in here and must use
+ EnqueueLocked.
+
+ The MotionNotify compression we do can't work across threads
+ (without locking the entire queue, perhaps) since we call
+ hwnd.Queue.Peek, so we just punt and don't compress motion events
+ unless the owning thread is the one which got the X event.
+
+ ConfigureNotify is another fun one, since it modifies the hwnd's
+ bounds and then enqueues the event. We add a lock to Hwnd which
+ is held when setting configure_pending to true (and enqueuing the
+ event).
+
+ There is a race wrt the wake socket. we need to make sure that
+ only 1 thread is waiting on that socket, or else a thread could
+ sleep waiting for data that never comes. It's difficult (but not
+ impossible) to make happen, because it seems to require something
+ like the following:
+
+ 1. Thread 1 polls on wake_receive
+
+ 2. poll returns saying there's data to be read on
+ wake_receive.
+
+ 3. Thread 2 polls on wake_receive and immediately returns
+ saying there's data to be read.
+
+ 4. Thread 2 reads the wakeup byte from wake_receive
+
+ 5. Thread 1 attempts to read the wakeup byte from
+ wake_receive.
+
+ 6. Thread 2 exits (due to a form closing, perhaps).
+
+ 7. Thread 1 blocks forever.
+
+ Fun, eh?
+
+ Fixing the Expose handling isn't done yet, and the races inherent
+ in that piece of code are responsible for the drawing mistakes you
+ see when generating expose events in a MT app (like NPlot). This
+ one is the likely to be the hardest to bandaid, and it doesn't
+ appear to cause anything but drawing problems. The other issues
+ caused apps to exit or hang.
+
+ * XEventQueue.cs: output some spew when Dequeue/Enqueue/Peek are
+ called from a different thread than the one that should be calling
+ these functions.
+
+ * Hwnd.cs: add some locks to be used by the XplatUIX11 code.
+
2006-11-15 Chris Toshok <[EMAIL PROTECTED]>
* Application.cs: null out the context's MainForm when we exit
Modified: trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/Hwnd.cs
===================================================================
--- trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/Hwnd.cs
2006-11-16 01:54:29 UTC (rev 67971)
+++ trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/Hwnd.cs
2006-11-16 05:45:58 UTC (rev 67972)
@@ -79,6 +79,10 @@
internal bool no_activate; // For Win32, popup
windows will not steal focus
#endregion // Local Variables
+ // locks for some operations (used in XplatUIX11.cs)
+ internal object configure_lock = new object ();
+ internal object expose_lock = new object ();
+
#region Constructors and destructors
public Hwnd() {
x = 0;
Modified:
trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/XEventQueue.cs
===================================================================
--- trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/XEventQueue.cs
2006-11-16 01:54:29 UTC (rev 67971)
+++ trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/XEventQueue.cs
2006-11-16 05:45:58 UTC (rev 67972)
@@ -78,6 +78,11 @@
public void Enqueue (XEvent xevent)
{
+ if (Thread.CurrentThread != thread) {
+ Console.WriteLine ("Hwnd.Queue.Enqueue called
from a different thread without locking.");
+ Console.WriteLine (Environment.StackTrace);
+ }
+
xqueue.Enqueue (xevent);
}
@@ -90,6 +95,11 @@
public XEvent Dequeue ()
{
+ if (Thread.CurrentThread != thread) {
+ Console.WriteLine ("Hwnd.Queue.Dequeue called
from a different thread without locking.");
+ Console.WriteLine (Environment.StackTrace);
+ }
+
if (xqueue.Count == 0) {
lock (lqueue) {
return lqueue.Dequeue ();
@@ -100,6 +110,11 @@
public XEvent Peek()
{
+ if (Thread.CurrentThread != thread) {
+ Console.WriteLine ("Hwnd.Queue.Peek called from
a different thread without locking.");
+ Console.WriteLine (Environment.StackTrace);
+ }
+
if (xqueue.Count == 0) {
lock (lqueue) {
return lqueue.Peek ();
Modified:
trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/XplatUIX11.cs
===================================================================
--- trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/XplatUIX11.cs
2006-11-16 01:54:29 UTC (rev 67971)
+++ trunk/mcs/class/Managed.Windows.Forms/System.Windows.Forms/XplatUIX11.cs
2006-11-16 05:45:58 UTC (rev 67972)
@@ -100,6 +100,8 @@
private static Hashtable MessageQueues; //
Holds our thread-specific XEventQueues
#if __MonoCS__ //
private static Pollfd[] pollfds; // For
watching the X11 socket
+ private static bool wake_waiting;
+ private static object wake_waiting_lock = new object ();
#endif //
private static X11Keyboard Keyboard; //
private static X11Dnd Dnd;
@@ -1238,13 +1240,17 @@
}
}
}
+
+ // XXX this sucks. this isn't thread safe
hwnd.width = xevent.ConfigureEvent.width;
hwnd.height = xevent.ConfigureEvent.height;
hwnd.ClientRect = Rectangle.Empty;
- if (!hwnd.configure_pending) {
- hwnd.Queue.Enqueue(xevent);
- hwnd.configure_pending = true;
+ lock (hwnd.configure_lock) {
+ if (!hwnd.configure_pending) {
+ hwnd.Queue.EnqueueLocked
(xevent);
+ hwnd.configure_pending = true;
+ }
}
}
// We drop configure events for Client windows
@@ -1367,10 +1373,19 @@
if (timeout > 0) {
#if __MonoCS__
- Syscall.poll (pollfds, (uint)
pollfds.Length, timeout);
+ int length = pollfds.Length - 1;
+ lock (wake_waiting_lock) {
+ if (wake_waiting == false) {
+ length ++;
+ wake_waiting = true;
+ }
+ }
+
+ Syscall.poll (pollfds, (uint)length,
timeout);
// Clean out buffer, so we're not
busy-looping on the same data
if (pollfds[1].revents != 0) {
wake_receive.Receive(network_buffer, 0, 1, SocketFlags.None);
+ wake_waiting = false;
}
#endif
lock (XlibLock) {
@@ -1551,7 +1566,8 @@
case XEventName.MotionNotify: {
XEvent peek;
- if (hwnd.Queue.Count > 0) {
+ /* we can't do motion compression
across threads, so just punt if we don't match up */
+ if (Thread.CurrentThread ==
hwnd.Queue.Thread && hwnd.Queue.Count > 0) {
peek = hwnd.Queue.Peek();
if (peek.AnyEvent.type ==
XEventName.MotionNotify) {
continue;
@@ -1571,7 +1587,7 @@
case XEventName.FocusOut:
case XEventName.ClientMessage:
case XEventName.ReparentNotify:
- hwnd.Queue.Enqueue (xevent);
+ hwnd.Queue.EnqueueLocked (xevent);
break;
case XEventName.ConfigureNotify:
@@ -3564,8 +3580,10 @@
Console.WriteLine("GetMessage(): Window {0:X} ConfigureNotify x={1} y={2}
width={3} height={4}", hwnd.client_window.ToInt32(), xevent.ConfigureEvent.x,
xevent.ConfigureEvent.y, xevent.ConfigureEvent.width,
xevent.ConfigureEvent.height);
#endif
// if ((hwnd.x !=
xevent.ConfigureEvent.x) || (hwnd.y != xevent.ConfigureEvent.y) || (hwnd.width
!= xevent.ConfigureEvent.width) || (hwnd.height !=
xevent.ConfigureEvent.height)) {
- SendMessage(msg.hwnd,
Msg.WM_WINDOWPOSCHANGED, IntPtr.Zero, IntPtr.Zero);
- hwnd.configure_pending
= false;
+ lock
(hwnd.configure_lock) {
+
SendMessage(msg.hwnd, Msg.WM_WINDOWPOSCHANGED, IntPtr.Zero, IntPtr.Zero);
+
hwnd.configure_pending = false;
+ }
// We need to adjust
our client window to track the resize of whole_window
if (hwnd.whole_window
!= hwnd.client_window)
@@ -4121,7 +4139,7 @@
xevent.ClientMessageEvent.ptr3 = wparam;
xevent.ClientMessageEvent.ptr4 = lparam;
- hwnd.Queue.Enqueue (xevent);
+ hwnd.Queue.EnqueueLocked (xevent);
return true;
}
@@ -4510,7 +4528,7 @@
xevent.MotionEvent.y_root = root_y;
xevent.MotionEvent.state = mask;
- child_hwnd.Queue.Enqueue (xevent);
+ child_hwnd.Queue.EnqueueLocked (xevent);
}
} else {
Hwnd hwnd;
_______________________________________________
Mono-patches maillist - [email protected]
http://lists.ximian.com/mailman/listinfo/mono-patches
