[Mono-winforms-list] [PATCH] Additional locking in Carbon XplatUI driver

[Dick Porter]

Hi All

I've observed NullReferenceExceptions from
System.Windows.Forms.XplatUICarbon.GetMessage() (specifically,
MessageQueue.Dequeue() inside GetMessage was randomly returning nulls)
and tracked it down to the locking around MessageQueue.  This patch
ensures that MessageQueue is locked whenever it is modified, completely
curing the exceptions for me.

Is this OK to check into HEAD?

- Dick


Index: class/Managed.Windows.Forms/System.Windows.Forms/ChangeLog
===================================================================
--- class/Managed.Windows.Forms/System.Windows.Forms/ChangeLog	(revision 133032)
+++ class/Managed.Windows.Forms/System.Windows.Forms/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2009-04-29  Dick Porter  <d...@acm.org>
+
+	* XplatUICarbon.cs: Add more locking around MessageQueue
+	manipulations.
+
 2009-04-27  Andrés G. Aragoneses  <aaragone...@novell.com>
 
 	* ListView.cs: Make OnColumnClick +internal to be used by a11y.
Index: class/Managed.Windows.Forms/System.Windows.Forms/XplatUICarbon.cs
===================================================================
--- class/Managed.Windows.Forms/System.Windows.Forms/XplatUICarbon.cs	(revision 133032)
+++ class/Managed.Windows.Forms/System.Windows.Forms/XplatUICarbon.cs	(working copy)
@@ -141,13 +141,15 @@
 
 		internal void FlushQueue () {
 			CheckTimers (DateTime.UtcNow);
-			while (MessageQueue.Count > 0) {
-				object queueobj = MessageQueue.Dequeue ();
-				if (queueobj is GCHandle) {
-					XplatUIDriverSupport.ExecuteClientMessage((GCHandle)queueobj);
-				} else {
-					MSG msg = (MSG)queueobj;
-					NativeWindow.WndProc (msg.hwnd, msg.message, msg.wParam, msg.lParam);
+			lock (queuelock) {
+				while (MessageQueue.Count > 0) {
+					object queueobj = MessageQueue.Dequeue ();
+					if (queueobj is GCHandle) {
+						XplatUIDriverSupport.ExecuteClientMessage((GCHandle)queueobj);
+					} else {
+						MSG msg = (MSG)queueobj;
+						NativeWindow.WndProc (msg.hwnd, msg.message, msg.wParam, msg.lParam);
+					}
 				}
 			}
 		}
@@ -420,7 +422,7 @@
 				msg.message = Msg.WM_MOUSEHOVER;
 				msg.wParam = GetMousewParam (0);
 				msg.lParam = (IntPtr)((ushort)Hover.X << 16 | (ushort)Hover.X);
-				MessageQueue.Enqueue (msg);
+				EnqueueMessage (msg);
 			}
 		}
 		#endregion
@@ -736,7 +738,7 @@
 					MSG msg = new MSG ();
 					msg.message = Msg.WM_PAINT;
 					msg.hwnd = hwnd.Handle;
-					MessageQueue.Enqueue (msg);
+					EnqueueMessage (msg);
 					hwnd.expose_pending = true;
 				}
 			} else {
@@ -749,7 +751,7 @@
 					msg.wParam = hrgn == IntPtr.Zero ? (IntPtr)1 : hrgn;
 					msg.refobject = rgn;
 					msg.hwnd = hwnd.Handle;
-					MessageQueue.Enqueue (msg);
+					EnqueueMessage (msg);
 					hwnd.nc_expose_pending = true;
 
 				}
@@ -1613,7 +1615,7 @@
 			msg.message = message;
 			msg.wParam = wParam;
 			msg.lParam = lParam;
-			MessageQueue.Enqueue (msg);
+			EnqueueMessage (msg);
 			return true;
 		}
 
@@ -1681,7 +1683,9 @@
 		[MonoTODO]
 		internal override void SendAsyncMethod (AsyncMethodData method) {
 			// Fake async
-			MessageQueue.Enqueue (GCHandle.Alloc (method));
+			lock (queuelock) {
+				MessageQueue.Enqueue (GCHandle.Alloc (method));
+			}
 		}
 
 		[MonoTODO]

signature.asc
Description: This is a digitally signed message part

_______________________________________________
Mono-winforms-list maillist  -  Mono-winforms-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-winforms-list