Crash on Mac OS when specifying wrong window size

[hiker]

Hi,

in STK we recently had the following crash report:

Code: Select all

#1	0x943b60c1 in -[NSWindow _confirmSize:force:]
#2	0x9424014f in _NXMakeWindowVisible
#3	0x9423faa7 in -[NSWindow constrainFrameRect:toScreen:]
#4	0x9422b997 in -[NSWindow _reallyDoOrderWindow:relativeTo:findKey:forCounter:force:isModal:]
#5	0x9422b64d in -[NSWindow orderWindow:relativeTo:]
#6	0x94228e03 in -[NSWindow makeKeyAndOrderFront:]
#7	0x004f85a1 in irr::CIrrDeviceMacOSX::createWindow
#8	0x004fca09 in irr::CIrrDeviceMacOSX::CIrrDeviceMacOSX
#9	0x0048062a in createDeviceEx at input.hpp:140
#10	0x00480762 in createDevice at input.hpp:140
#11	0x002131d2 in IrrDriver::initDevice at irr_driver.cpp:164
#12	0x00213d5b in IrrDriver::IrrDriver at irr_driver.cpp:57
#13	0x0013fb00 in initRest at main.cpp:544
#14	0x00142066 in main at main.cpp:637

Apparently the user was using a laptop, which was sometimes connected to a large screen. When he tried to use the laptop without the external screen, irrlicht was not able to create the device, since it was requesting a too large window size. We were able to reproduce the crash by just requesting some large window size (using OpenGL).

Could this crash be prevented (if possible in 1.7.2)? E.g. returning a NULL from createDevice would enable us to handle a situation like this. Note that an invalid screen size does apparently not cause any problems on windows or linux.

Thanks!
Joerg

[CuteAlien]

Anyone who can do a patch for that? Getting it in 1.7.2 might be tricky as that should be released in a few days (would have been released already if someone would have found the time for it the last days).

[hiker]

Hi,

Anyone who can do a patch for that? Getting it in 1.7.2 might be tricky as that should be released in a few days (would have been released already if someone would have found the time for it the last days).

ah, sorry - I wasn't aware that you are that close to a release. No need to postpone your release (or risk a not thoroughly tested patch) for this bug, since we already support command line options to change the resolution (though of course that's not really nice, and a fix here would be good).

Thanks a lot!
Joerg

[hybrid]

This is a known bug: http://sourceforge.net/tracker/?func=de ... tid=540676
Unfortunately, no one here works with OSX, and even fewer ones over here would know how to do this under OSX. Maybe someone can provide a patch?

[hiker]

Hi,

This is a known bug: http://sourceforge.net/tracker/?func=de ... tid=540676
Unfortunately, no one here works with OSX, and even fewer ones over here would know how to do this under OSX. Maybe someone can provide a patch?

thanks, I've missed that report. I've posted in the German 'games 4 mac' forum:
http://community.games4mac.de/index.php?showtopic=21167.

Perhaps they can help.

Cheers,
Joerg

[Auria]

FYI: I posted a patch on https://sourceforge.net/tracker/?func=d ... tid=540676

(also saving it here in case the pastebin gets lost)

Code: Select all

Index: source/Irrlicht/Irrlicht.cpp
===================================================================
--- source/Irrlicht/Irrlicht.cpp	(revision 3409)
+++ source/Irrlicht/Irrlicht.cpp	(working copy)
@@ -3,6 +3,7 @@
 // For conditions of distribution and use, see copyright notice in irrlicht.h
 
 #include "IrrCompileConfig.h"
+#include <stdexcept>
 
 static const char* const copyright = "Irrlicht Engine (c) 2002-2009 Nikolaus Gebhardt";
 
@@ -74,7 +75,16 @@
 
 #ifdef _IRR_COMPILE_WITH_OSX_DEVICE_
 		if (params.DeviceType == EIDT_OSX || (!dev && params.DeviceType == EIDT_BEST))
-			dev = new CIrrDeviceMacOSX(params);
+        {
+            try
+            {
+                dev = new CIrrDeviceMacOSX(params);
+            }
+            catch (std::runtime_error& e)
+            {
+                return NULL;
+            }
+        }
 #endif
 
 #ifdef _IRR_COMPILE_WITH_WINDOWS_CE_DEVICE_
Index: source/Irrlicht/MacOSX/CIrrDeviceMacOSX.mm
===================================================================
--- source/Irrlicht/MacOSX/CIrrDeviceMacOSX.mm	(revision 3409)
+++ source/Irrlicht/MacOSX/CIrrDeviceMacOSX.mm	(working copy)
@@ -9,6 +9,7 @@
 #import <Cocoa/Cocoa.h>
 #import <OpenGL/gl.h>
 #import <Carbon/Carbon.h>
+#include <stdexcept>
 
 #include "CIrrDeviceMacOSX.h"
 #include "IEventReceiver.h"
@@ -370,7 +371,13 @@
 
 	initKeycodes();
 	if (CreationParams.DriverType != video::EDT_NULL)
-		createWindow();
+    {
+		const bool success = createWindow();
+        if (!success)
+        {
+            throw std::runtime_error("Cannot create video device");
+        }
+    }
 
 	setResizable(false);
 
@@ -458,171 +465,180 @@
 
 	VideoModeList.setDesktop(CreationParams.Bits, core::dimension2d<u32>(ScreenWidth, ScreenHeight));
 
-	if (!CreationParams.Fullscreen)
-	{
-		if(!CreationParams.WindowId) //create another window when WindowId is null
-		{
-			Window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0,0,CreationParams.WindowSize.Width,CreationParams.WindowSize.Height) styleMask:NSTitledWindowMask+NSClosableWindowMask+NSResizableWindowMask backing:NSBackingStoreBuffered defer:FALSE];
-		}
+    @try
+    {
+        if (!CreationParams.Fullscreen)
+        {
+            if(!CreationParams.WindowId) //create another window when WindowId is null
+            {
+                Window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0,0,CreationParams.WindowSize.Width,CreationParams.WindowSize.Height) styleMask:NSTitledWindowMask+NSClosableWindowMask+NSResizableWindowMask backing:NSBackingStoreBuffered defer:FALSE];
+            }
 
-		if (Window != NULL || CreationParams.WindowId)
-		{
-			NSOpenGLPixelFormatAttribute windowattribs[] =
-			{
-					NSOpenGLPFANoRecovery,
-					NSOpenGLPFAAccelerated,
-					NSOpenGLPFADepthSize,     (NSOpenGLPixelFormatAttribute)depthSize,
-					NSOpenGLPFAColorSize,     (NSOpenGLPixelFormatAttribute)CreationParams.Bits,
-					NSOpenGLPFAAlphaSize,     (NSOpenGLPixelFormatAttribute)alphaSize,
-					NSOpenGLPFASampleBuffers, (NSOpenGLPixelFormatAttribute)1,
-					NSOpenGLPFASamples,       (NSOpenGLPixelFormatAttribute)CreationParams.AntiAlias,
-					NSOpenGLPFAStencilSize,   (NSOpenGLPixelFormatAttribute)(CreationParams.Stencilbuffer?1:0),
-					NSOpenGLPFADoubleBuffer,
-					(NSOpenGLPixelFormatAttribute)nil
-			};
+            if (Window != NULL || CreationParams.WindowId)
+            {
+                NSOpenGLPixelFormatAttribute windowattribs[] =
+                {
+                        NSOpenGLPFANoRecovery,
+                        NSOpenGLPFAAccelerated,
+                        NSOpenGLPFADepthSize,     (NSOpenGLPixelFormatAttribute)depthSize,
+                        NSOpenGLPFAColorSize,     (NSOpenGLPixelFormatAttribute)CreationParams.Bits,
+                        NSOpenGLPFAAlphaSize,     (NSOpenGLPixelFormatAttribute)alphaSize,
+                        NSOpenGLPFASampleBuffers, (NSOpenGLPixelFormatAttribute)1,
+                        NSOpenGLPFASamples,       (NSOpenGLPixelFormatAttribute)CreationParams.AntiAlias,
+                        NSOpenGLPFAStencilSize,   (NSOpenGLPixelFormatAttribute)(CreationParams.Stencilbuffer?1:0),
+                        NSOpenGLPFADoubleBuffer,
+                        (NSOpenGLPixelFormatAttribute)nil
+                };
 
-			if (CreationParams.AntiAlias<2)
-			{
-				windowattribs[ 9] = (NSOpenGLPixelFormatAttribute)0;
-				windowattribs[11] = (NSOpenGLPixelFormatAttribute)0;
-			}
+                if (CreationParams.AntiAlias<2)
+                {
+                    windowattribs[ 9] = (NSOpenGLPixelFormatAttribute)0;
+                    windowattribs[11] = (NSOpenGLPixelFormatAttribute)0;
+                }
 
-			NSOpenGLPixelFormat *format;
-			for (int i=0; i<3; ++i)
-			{
-				if (1==i)
-				{
-					// Second try without stencilbuffer
-					if (CreationParams.Stencilbuffer)
-					{
-						windowattribs[13]=(NSOpenGLPixelFormatAttribute)0;
-					}
-					else
-						continue;
-				}
-				else if (2==i)
-				{
-					// Third try without Doublebuffer
-					os::Printer::log("No doublebuffering available.", ELL_WARNING);
-					windowattribs[14]=(NSOpenGLPixelFormatAttribute)nil;
-				}
+                NSOpenGLPixelFormat *format;
+                for (int i=0; i<3; ++i)
+                {
+                    if (1==i)
+                    {
+                        // Second try without stencilbuffer
+                        if (CreationParams.Stencilbuffer)
+                        {
+                            windowattribs[13]=(NSOpenGLPixelFormatAttribute)0;
+                        }
+                        else
+                            continue;
+                    }
+                    else if (2==i)
+                    {
+                        // Third try without Doublebuffer
+                        os::Printer::log("No doublebuffering available.", ELL_WARNING);
+                        windowattribs[14]=(NSOpenGLPixelFormatAttribute)nil;
+                    }
 
-				format = [[NSOpenGLPixelFormat alloc] initWithAttributes:windowattribs];
-				if (format == NULL)
-				{
-					if (CreationParams.AntiAlias>1)
-					{
-						while (!format && windowattribs[12]>1)
-						{
-							windowattribs[12] = (NSOpenGLPixelFormatAttribute)((int)windowattribs[12]-1);
-							format = [[NSOpenGLPixelFormat alloc] initWithAttributes:windowattribs];
-						}
+                    format = [[NSOpenGLPixelFormat alloc] initWithAttributes:windowattribs];
+                    if (format == NULL)
+                    {
+                        if (CreationParams.AntiAlias>1)
+                        {
+                            while (!format && windowattribs[12]>1)
+                            {
+                                windowattribs[12] = (NSOpenGLPixelFormatAttribute)((int)windowattribs[12]-1);
+                                format = [[NSOpenGLPixelFormat alloc] initWithAttributes:windowattribs];
+                            }
 
-						if (!format)
-						{
-							windowattribs[9] = (NSOpenGLPixelFormatAttribute)0;
-							windowattribs[11] = (NSOpenGLPixelFormatAttribute)0;
-							format = [[NSOpenGLPixelFormat alloc] initWithAttributes:windowattribs];
-							if (!format)
-							{
-								// reset values for next try
-								windowattribs[9] = (NSOpenGLPixelFormatAttribute)1;
-								windowattribs[11] = (NSOpenGLPixelFormatAttribute)CreationParams.AntiAlias;
-							}
-							else
-							{
-								os::Printer::log("No FSAA available.", ELL_WARNING);
-							}
+                            if (!format)
+                            {
+                                windowattribs[9] = (NSOpenGLPixelFormatAttribute)0;
+                                windowattribs[11] = (NSOpenGLPixelFormatAttribute)0;
+                                format = [[NSOpenGLPixelFormat alloc] initWithAttributes:windowattribs];
+                                if (!format)
+                                {
+                                    // reset values for next try
+                                    windowattribs[9] = (NSOpenGLPixelFormatAttribute)1;
+                                    windowattribs[11] = (NSOpenGLPixelFormatAttribute)CreationParams.AntiAlias;
+                                }
+                                else
+                                {
+                                    os::Printer::log("No FSAA available.", ELL_WARNING);
+                                }
 
-						}
-					}
-				}
-				else
-					break;
-			}
-			CreationParams.AntiAlias = windowattribs[11];
-			CreationParams.Stencilbuffer=(windowattribs[13]==1);
+                            }
+                        }
+                    }
+                    else
+                        break;
+                }
+                CreationParams.AntiAlias = windowattribs[11];
+                CreationParams.Stencilbuffer=(windowattribs[13]==1);
 
-			if (format != NULL)
-			{
-				OGLContext = [[NSOpenGLContext alloc] initWithFormat:format shareContext:NULL];
-				[format release];
-			}
+                if (format != NULL)
+                {
+                    OGLContext = [[NSOpenGLContext alloc] initWithFormat:format shareContext:NULL];
+                    [format release];
+                }
 
-			if (OGLContext != NULL)
-			{
-				if (!CreationParams.WindowId)
-				{
-					[Window center];
-					[Window setDelegate:[NSApp delegate]];
-					[OGLContext setView:[Window contentView]];
-					[Window setAcceptsMouseMovedEvents:TRUE];
-					[Window setIsVisible:TRUE];
-					[Window makeKeyAndOrderFront:nil];
-				}
-				else //use another window for drawing
-					[OGLContext setView:(NSView*)CreationParams.WindowId];
+                if (OGLContext != NULL)
+                {
+                    if (!CreationParams.WindowId)
+                    {
+                        [Window center];
+                        [Window setDelegate:[NSApp delegate]];
+                        [OGLContext setView:[Window contentView]];
+                        [Window setAcceptsMouseMovedEvents:TRUE];
+                        [Window setIsVisible:TRUE];
+                        [Window makeKeyAndOrderFront:nil];
+                    }
+                    else //use another window for drawing
+                        [OGLContext setView:(NSView*)CreationParams.WindowId];
 
-				CGLContext = (CGLContextObj) [OGLContext CGLContextObj];
-				DeviceWidth = CreationParams.WindowSize.Width;
-				DeviceHeight = CreationParams.WindowSize.Height;
-				result = true;
-			}
-		}
-	}
-	else
-	{
-		displaymode = CGDisplayBestModeForParameters(display,CreationParams.Bits,CreationParams.WindowSize.Width,CreationParams.WindowSize.Height,NULL);
-		if (displaymode != NULL)
-		{
-			olddisplaymode = CGDisplayCurrentMode(display);
-			error = CGCaptureAllDisplays();
-			if (error == CGDisplayNoErr)
-			{
-				error = CGDisplaySwitchToMode(display,displaymode);
-				if (error == CGDisplayNoErr)
-				{
-					pixelFormat = NULL;
-					numPixelFormats = 0;
+                    CGLContext = (CGLContextObj) [OGLContext CGLContextObj];
+                    DeviceWidth = CreationParams.WindowSize.Width;
+                    DeviceHeight = CreationParams.WindowSize.Height;
+                    result = true;
+                }
+            }
+        }
+        else
+        {
+            displaymode = CGDisplayBestModeForParameters(display,CreationParams.Bits,CreationParams.WindowSize.Width,CreationParams.WindowSize.Height,NULL);
+            if (displaymode != NULL)
+            {
+                olddisplaymode = CGDisplayCurrentMode(display);
+                error = CGCaptureAllDisplays();
+                if (error == CGDisplayNoErr)
+                {
+                    error = CGDisplaySwitchToMode(display,displaymode);
+                    if (error == CGDisplayNoErr)
+                    {
+                        pixelFormat = NULL;
+                        numPixelFormats = 0;
 
-					int index = 0;
-					CGLPixelFormatAttribute	fullattribs[] =
-					{
-						kCGLPFAFullScreen,
-						kCGLPFADisplayMask, (CGLPixelFormatAttribute)CGDisplayIDToOpenGLDisplayMask(display),
-						kCGLPFADoubleBuffer,
-						kCGLPFAAccelerated,
-						kCGLPFADepthSize, (CGLPixelFormatAttribute)depthSize,
-						kCGLPFAColorSize, (CGLPixelFormatAttribute)CreationParams.Bits,
-						kCGLPFAAlphaSize, (CGLPixelFormatAttribute)alphaSize,
-						kCGLPFASampleBuffers, (CGLPixelFormatAttribute)(CreationParams.AntiAlias?1:0),
-						kCGLPFASamples, (CGLPixelFormatAttribute)CreationParams.AntiAlias,
-						kCGLPFAStencilSize, (CGLPixelFormatAttribute)(CreationParams.Stencilbuffer?1:0),
-						(CGLPixelFormatAttribute)NULL
-					};
+                        int index = 0;
+                        CGLPixelFormatAttribute	fullattribs[] =
+                        {
+                            kCGLPFAFullScreen,
+                            kCGLPFADisplayMask, (CGLPixelFormatAttribute)CGDisplayIDToOpenGLDisplayMask(display),
+                            kCGLPFADoubleBuffer,
+                            kCGLPFAAccelerated,
+                            kCGLPFADepthSize, (CGLPixelFormatAttribute)depthSize,
+                            kCGLPFAColorSize, (CGLPixelFormatAttribute)CreationParams.Bits,
+                            kCGLPFAAlphaSize, (CGLPixelFormatAttribute)alphaSize,
+                            kCGLPFASampleBuffers, (CGLPixelFormatAttribute)(CreationParams.AntiAlias?1:0),
+                            kCGLPFASamples, (CGLPixelFormatAttribute)CreationParams.AntiAlias,
+                            kCGLPFAStencilSize, (CGLPixelFormatAttribute)(CreationParams.Stencilbuffer?1:0),
+                            (CGLPixelFormatAttribute)NULL
+                        };
 
-					CGLChoosePixelFormat(fullattribs,&pixelFormat,&numPixelFormats);
+                        CGLChoosePixelFormat(fullattribs,&pixelFormat,&numPixelFormats);
 
-					if (pixelFormat != NULL)
-					{
-						CGLCreateContext(pixelFormat,NULL,&CGLContext);
-						CGLDestroyPixelFormat(pixelFormat);
-					}
+                        if (pixelFormat != NULL)
+                        {
+                            CGLCreateContext(pixelFormat,NULL,&CGLContext);
+                            CGLDestroyPixelFormat(pixelFormat);
+                        }
 
-					if (CGLContext != NULL)
-					{
-						CGLSetFullScreen(CGLContext);
-						displayRect = CGDisplayBounds(display);
-						ScreenWidth = DeviceWidth = (int)displayRect.size.width;
-						ScreenHeight = DeviceHeight = (int)displayRect.size.height;
-						CreationParams.WindowSize.set(ScreenWidth, ScreenHeight);
-						result = true;
-					}
-				}
-			}
-		}
-	}
+                        if (CGLContext != NULL)
+                        {
+                            CGLSetFullScreen(CGLContext);
+                            displayRect = CGDisplayBounds(display);
+                            ScreenWidth = DeviceWidth = (int)displayRect.size.width;
+                            ScreenHeight = DeviceHeight = (int)displayRect.size.height;
+                            CreationParams.WindowSize.set(ScreenWidth, ScreenHeight);
+                            result = true;
+                        }
+                    }
+                }
+            }
+        }
 
+    }
+    @catch (NSException *exception)
+    {
+        // FIXME: cleanup may be needed here
+        return false;
+    }
+    
 	if (result)
 	{
 		// fullscreen?

[Auria]

Friendly bump
The current patch is not perfect, in that in can slightly leak when an error occurs (not in normal cases), but IMHO leaking a little on errors is much, much better than crashing
The patch seems huge, but this is only because indentation changed; the only change was to surround the code with try...catch. So I would appreciate if this could be applied (catching exceptions so they don't crash the entire program can't break anything AFAICT)

[hybrid]

Yeah, sorry. I will test and commit this patch once I get hands on my Mac again. This is always a little longer than for any other OS, but I will make sure that it goes into Irrlicht 1.8 for sure, maybe even 1.7.3

[xDan]

The suggested fix, as far as I can see, just causes the error to be caught. But it would be nice to try a valid window size instead.

So what about this:

In CIrrDeviceMacOSX.mm,

replace

Code: Select all

                Window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0,0,CreationParams.WindowSize.Width,CreationParams.WindowSize.Height) styleMask:NSTitledWindowMask+NSClosableWindowMask+NSResizableWindowMask backing:NSBackingStoreBuffered defer:FALSE];
 

with

Code: Select all

                NSRect desiredRect = NSMakeRect(0,0,CreationParams.WindowSize.Width,CreationParams.WindowSize.Height);
                NSRect usableScreenRect = [[NSScreen mainScreen] visibleFrame];
                
                if (usableScreenRect.origin.x > 0.f)
                {
                        desiredRect.origin.x = usableScreenRect.origin.x;
                        os::Printer::log( (core::stringc("Usable screen origin.x is not 0, adjusted to: ")
                                        + core::stringc(desiredRect.origin.x)).c_str(), ELL_WARNING);
                }
                
                if (usableScreenRect.origin.y > 0.f)
                {
                        desiredRect.origin.y = usableScreenRect.origin.y;
                        os::Printer::log( (core::stringc("Usable screen origin.y is not 0, adjusted to: ")
                                        + core::stringc(desiredRect.origin.y)).c_str(), ELL_WARNING);
                }
                
                if (usableScreenRect.size.width < desiredRect.size.width)
                {
                        desiredRect.size.width = usableScreenRect.size.width;
                        os::Printer::log( (core::stringc("Desired screen width is too large, adjusted to: ")
                                        + core::stringc(desiredRect.size.width)).c_str(), ELL_WARNING);
                }
                
                if (usableScreenRect.size.height < desiredRect.size.height)
                {
                        desiredRect.size.height = usableScreenRect.size.height;
                        os::Printer::log( (core::stringc("Desired screen height is too large, adjusted to: ")
                                        + core::stringc(desiredRect.size.height)).c_str(), ELL_WARNING);
                }
                
                // Update CreationParams for Driver and other uses.
                CreationParams.WindowSize.Width = (u32)desiredRect.size.width;
                CreationParams.WindowSize.Height = (u32)desiredRect.size.height;
                
                Window = [[NSWindow alloc] initWithContentRect:desiredRect styleMask:NSTitledWindowMask+NSClosableWindowMask+NSResizableWindowMask backing:NSBackingStoreBuffered defer:FALSE];
 

(seems to fix the problem for me.... not extensively tested though. Irrlicht 1.6.1)

[hybrid]

Ok, thanks. Unfortunately, the last two times I started on OSX many other things came up first. But I hope to come through and test this one any time soon.