Bug 159 - Using NSWindow events cause a crash when using sheets
Summary: Using NSWindow events cause a crash when using sheets
Status: RESOLVED FIXED
Alias: None
Product: MonoMac
Classification: Desktop
Component: Bindings ()
Version: GIT
Hardware: Macintosh Mac OS
: --- normal
Target Milestone: ---
Assignee: Rolf Bjarne Kvinge [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2011-08-03 04:43 UTC by Curtis Wensley
Modified: 2012-02-13 18:08 UTC (History)
3 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Sample application to reproduce the issue (16.23 KB, application/zip)
2011-08-03 04:43 UTC, Curtis Wensley
Details
Proposed patch (11.66 KB, patch)
2012-02-13 11:54 UTC, Rolf Bjarne Kvinge [MSFT]
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Curtis Wensley 2011-08-03 04:43:07 UTC
Created attachment 55 [details]
Sample application to reproduce the issue

When using NSWindow events causes a crash when trying to present an NSAlert, NSOpenPanel, etc in a sheet.

This seems to stem from the WillPositionSheet() method of the delegate.

Attached is a test application that reproduces the issue both when using events, or when implementing the above method in a delegate manually.
Comment 1 Curtis Wensley 2011-08-08 13:52:17 UTC
Note that with the attached sample, it will also crash if you try to zoom the window.
Comment 2 Curtis Wensley 2012-02-01 21:16:02 UTC
Any ideas on this issue?  I would definitely help out and fix it if I can (I've tried), but can't seem to find where the issue resides..
Comment 3 Miguel de Icaza [MSFT] 2012-02-08 23:47:06 UTC
Ok, both problems are caused by a signature bug in the NSWindowDelegate, now I need to track down which method is the responsible one.

Since we provide dispatchers for all events even if only one is overwritten, the search might take a while.
Comment 4 Curtis Wensley 2012-02-08 23:49:16 UTC
There's two:  WillPositionSheet and WillUseStandardFrame..

If I comment these two out, everything seems to work.

I think it's having issues marshalling the RectangleF as a return value, as these are the only API's that do so..
Comment 5 Miguel de Icaza [MSFT] 2012-02-09 00:12:31 UTC
Ah, thanks for debugging that.   I just came across these by doing a binary search by removing features on the NSWindowDelegate.

My guess is close to yours, that the calling conventions are probably wrong for RectangleF on return values.
Comment 6 Miguel de Icaza [MSFT] 2012-02-09 02:12:14 UTC
What I have found so far:

* Parameters reach the callback correctly, and the return value is correct.
* The StructureToPtr invocation gets the right information, both the return address and the object to marshal
* Disabling the invocation to the StructToPtr does not do anything, so we can rule out memory corruption due to the marshaling of the data out.
* Even disabling the entire method generation for the trampoline method (so our callback methods or overwritten method are not invoked) cause the code to crash.

This seems to indicate that the dynamically generated code in NativeMethodBuilder.cs and NativeImplementationBuilder.cs are correct and that the problem is somewhere else.

Some lines of investigation for tomorrow:

* Modify Class.cs's RegisterMethod to special case these callbacks and instead of passing a Delegate to the dynamic method, pass a delegate to a static method with the desired signature (special case the method we want to actually catch)
Comment 7 Miguel de Icaza [MSFT] 2012-02-09 16:57:47 UTC
Replacing the dynamically generated code with a static delegate still causes the problem:

https://gist.github.com/085aacca4c8d1092ab28
Comment 8 Rolf Bjarne Kvinge [MSFT] 2012-02-10 08:10:54 UTC
For WillPositionSheet the problem is very likely that we have to leave the stack unbalanced upon return. In MonoTouch we have custom assembly code for this case, and when we return from the method we do a "ret $0x4" instead of just "ret". This is how ObjC does it for functions that return structures with floating point members (which WillPositionSheet is).

However I have no idea how to tell Mono to jit methods this way. It also looks like we'll need to tell Mono somehow which methods should jit like this, since it's obviously only for ObjC methods and not C methods (and DynamicMethod doesn't seem to have any way to add custom attributes).
Comment 9 Rolf Bjarne Kvinge [MSFT] 2012-02-10 20:09:58 UTC
Here is a (very ugly) patch that works (for this particular test case): https://gist.github.com/1ec2d9610ef9543718c2

I'll clean it up and make it handle more cases next week.
Comment 10 Rolf Bjarne Kvinge [MSFT] 2012-02-13 11:54:20 UTC
Created attachment 1355 [details]
Proposed patch

Attaching a complete patch, should be ready to commit.
Comment 11 Rolf Bjarne Kvinge [MSFT] 2012-02-13 18:08:43 UTC
Patch commited: 18a98ee62.