[8] Review request for 8007146 [macosx] Setting a display mode crashes JDK under VNC

[Anthony Petrov]

Hi Alexander,

Given the remark about invalidity of the config on return of this 
function, I'd think a call to CFRelease is unneeded.

E.g. see http://forum.openframeworks.cc/index.php?topic=5175.0 - they 
don't CFRelease() anything.

--
best regards,
Anthony

On 2/1/2013 19:48, Alexander Scherbatiy wrote:
> 
>   Hi Anthony,
> 
>    The CGCompleteDisplayConfiguration documenation says that:
>   -------------------------------------------
>    configRef: The display configuration with the desired changes. On 
> return, this configuration is no longer valid.
>   -------------------------------------------
>    
> https://developer.apple.com/library/mac/#documentation/graphicsimaging/reference/Quartz_Services_Ref/Reference/reference.html 
> 
> 
>   Should the configRef be manually released after the 
> CGCompleteDisplayConfiguration method invocation?
>   Releasing the configRef crashes the system in case if the 
> CGCompleteDisplayConfiguration does not return successful result.
> 
>  Thanks,
>  Alexandr.
> 
> 
> On 1/31/2013 6:50 PM, Sergey Bylokhov wrote:
>> Hi, Alexander.
>> Thanks for info. Fix looks good to me. But I am not sure about 
>> CFRelease removing, because "On return, this configuration is no 
>> longer valid." is not clear.
>>
>> 31.01.2013 18:28, Alexander Scherbatiy wrote:
>>> On 1/31/2013 5:56 PM, Sergey Bylokhov wrote:
>>>> Hi, Alexander.
>>>> Could we check DM in setDisplayMode() against modes from 
>>>> CGraphicsDevice.getDisplayModes() on java lvl?
>>>     1. We need to filter display modes which have width = 1 and 
>>> height = 1 in the list returned by the CGDisplayCopyAllDisplayModes 
>>> function.
>>>    The code below reproduces the problem in pure Cocoa application:
>>>
>>> ------------------------------------------
>>> - (void) test {
>>>     int MAX_DISPLAYS = 100;
>>>     CGDirectDisplayID displays[MAX_DISPLAYS];
>>>     uint32_t numDisplays;
>>>     uint32_t i;
>>>     CGGetActiveDisplayList(MAX_DISPLAYS, displays, &numDisplays);
>>>     for(i=0; i<numDisplays; i++)
>>>     {
>>>         CGDisplayModeRef mode;
>>>         CFIndex index, count;
>>>         CFArrayRef modeList;
>>>         modeList=CGDisplayCopyAllDisplayModes(displays[i], NULL);
>>>         count=CFArrayGetCount(modeList);
>>>         NSLog(@"\n\nmode --");
>>>         for(index=0;index<count;index++)
>>>         {
>>> mode=(CGDisplayModeRef)CFArrayGetValueAtIndex(modeList, index);
>>>             long h=0, w=0;
>>>             h=CGDisplayModeGetHeight(mode);
>>>             w=CGDisplayModeGetWidth(mode);
>>>             uint32_t flags=CGDisplayModeGetIOFlags(mode);
>>>             NSLog(@"flags: %d", flags);
>>>             NSLog(@"w, h: %ld, %ld", w, h);
>>>         }
>>>         CFRelease(modeList);
>>>     }
>>>
>>> }
>>> ------------------------------------------------
>>> mode --
>>> 2013-01-31 18:29:35.220 MyWindowTest[78789:707] flags: 7
>>> 2013-01-31 18:29:35.221 MyWindowTest[78789:707] w, h: 1, 1
>>> 2013-01-31 18:29:35.221 MyWindowTest[78789:707] flags: 7
>>> 2013-01-31 18:29:35.222 MyWindowTest[78789:707] w, h: 1, 1
>>> 2013-01-31 18:29:35.222 MyWindowTest[78789:707] flags: 7
>>> 2013-01-31 18:29:35.223 MyWindowTest[78789:707] w, h: 1, 1
>>> 2013-01-31 18:29:35.223 MyWindowTest[78789:707] flags: 7
>>> 2013-01-31 18:29:35.223 MyWindowTest[78789:707] w, h: 1, 1
>>> 2013-01-31 18:29:35.224 MyWindowTest[78789:707] flags: 7
>>> 2013-01-31 18:29:35.224 MyWindowTest[78789:707] w, h: 1, 1
>>> --------------------------------------------------------------------------- 
>>>
>>>
>>>
>>> 2.  The getBestModeForParameters method allows to set only those 
>>> display methods which have the same width, height, and bit depth with 
>>> the existed display modes.
>>>    It seems that there is no need to check the display modes one more 
>>> time on java level.
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>>
>>>
>>>>
>>>> 31.01.2013 17:46, Alexander Scherbatiy wrote:
>>>>>
>>>>> bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8007146
>>>>> webrev: http://cr.openjdk.java.net/~alexsch/8007146/webrev.00
>>>>>
>>>>> The issue is reproduced when a program is running under a VNC and 
>>>>> all monitors are disconnected from the Mac OS X system.
>>>>> The CGDisplayCopyAllDisplayModes returns display modes with width 
>>>>> and height equal to 1.
>>>>> CGCompleteDisplayConfiguration returns error when the invalid 
>>>>> display mode is set and JDK crashes after releasing the display 
>>>>> configuration.
>>>>> According to the CGCompleteDisplayConfiguration doc: "On return, 
>>>>> this configuration is no longer valid."
>>>>>
>>>>> The fix filters the invalid display modes and does not release the 
>>>>> display configuration.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>
>>>>
>>>
>>
>>
>