libs-gui icon indicating copy to clipboard operation
libs-gui copied to clipboard

Crash in NSView from NSMenuView/NSView dealloc

Open rmottola opened this issue 9 months ago • 6 comments

On Solaris Sparc 64 bit on exit from SystemPreferences I get this crash, completely inside GUI. Needs display is invoked with a null rect, all comes from deallocing a menu with a nil. It is not happening in other application, this is very strange.

#0  0xfef4043c in -[NSView _setNeedsDisplayInRect:real:] (self=0x360d30, _cmd=<optimized out>, v=0x0)
    at NSView.m:2833
#1  0xfef3ec04 in -[NSView setNeedsDisplayInRect:] (self=0x360d30, 
    _cmd=0xff19aa18 <_OBJC_SELECTOR_TABLE+1536>, invalidRect=...) at NSView.m:2913
#2  0xfef32378 in -[NSView removeFromSuperview] (self=0x2cd360, 
    _cmd=0xff1769e0 <_OBJC_SELECTOR_TABLE+1536>) at NSView.m:939
#3  0xfee74ae8 in -[NSMenuView update] (self=0x360d30, _cmd=0xff176850 <_OBJC_SELECTOR_TABLE+1136>)
    at NSMenuView.m:682
#4  0xfee72d78 in -[NSMenuView setMenu:] (self=0x360d30, _cmd=<optimized out>, menu=<optimized out>)
    at NSMenuView.m:324
#5  0xfee6cb10 in -[NSMenu dealloc] (self=0x2adb30, _cmd=0xfe970838 <_OBJC_SELECTOR_TABLE+24>)
    at NSMenu.m:450
#6  0xfe65a070 in objc_release_fast_np_internal (anObject=0x2adb30) at NSObject.m:593
#7  release_fast (anObject=0x2adb30) at NSObject.m:607
#8  -[NSObject release] (self=0x2adb30, _cmd=0xfe96ddfc <_OBJC_SELECTOR_TABLE+40>) at NSObject.m:2112
#9  0xfe647574 in -[GSNotification dealloc] (self=0x35dae0, _cmd=0xfe970838 <_OBJC_SELECTOR_TABLE+24>)
    at NSNotificationCenter.m:101
#10 0xfe65a070 in objc_release_fast_np_internal (anObject=0x35dae0) at NSObject.m:593
#11 release_fast (anObject=0x35dae0) at NSObject.m:607
#12 -[NSObject release] (self=0x35dae0, _cmd=0xfe955204 <_OBJC_SELECTOR_TABLE+136>) at NSObject.m:2112
#13 0xfe578adc in -[NSAutoreleasePool emptyPool] (self=0xfead8, _cmd=<optimized out>)
    at NSAutoreleasePool.m:550
#14 0xfe578c30 in -[NSAutoreleasePool dealloc] (self=0xfead8, _cmd=0xfe99bf5c <_OBJC_SELECTOR_TABLE+80>)
    at NSAutoreleasePool.m:596
#15 0xfe79049c in handleExit () at NSObject+GNUstepBase.m:243
#16 0xfe2c31c0 in _exithandle () from /lib/libc.so.1
#17 0xfe2b1180 in exit () from /lib/libc.so.1
#18 0xfed87d00 in -[NSApplication replyToApplicationShouldTerminate:] (self=0x1880f8, 
    _cmd=<optimized out>, shouldTerminate=<optimized out>) at NSApplication.m:3621
#19 0xfed88d4c in -[NSApplication terminate:] (self=0x1880f8, 
    _cmd=0xff1ac034 <_OBJC_SELECTOR_TABLE+160>, sender=<optimized out>) at NSApplication.m:3567
#20 0xfed7f590 in -[NSApplication sendAction:to:from:] (self=self@entry=0x1880f8, 
    _cmd=_cmd@entry=0xff175608 <_OBJC_SELECTOR_TABLE+1248>, 
    aSelector=aSelector@entry=0xff1ac034 <_OBJC_SELECTOR_TABLE+160>, aTarget=<optimized out>, 
---Type <return> to continue, or q <return> to quit---

rmottola avatar Apr 16 '25 23:04 rmottola

A very strange thing.

In setNeedsDisplayInRect invalidRect is passed:

(gdb) p invalidRect 
$3 = {origin = {x = 0, y = 160}, size = {width = 172, height = 22}}

but

  v = [[NSValue alloc]
		 initWithBytes: &invalidRect
		 objCType: @encode(NSRect)];
(gdb) p v
$4 = (struct NSValue *) 0x0

so NSValue seems to fail to return something valid. We are within a dealloc method, that might screw up things.

rmottola avatar Apr 17 '25 00:04 rmottola

It seems though that NSValue in this dealloc context is not encoding the rect correctly, this is worrying. @rfm @fredkiefer

rmottola avatar Apr 17 '25 00:04 rmottola

#import <Foundation/Foundation.h>

int
main(int argc, const char *argv[])
{
  NSValue *v;
  NSRect invalidRect;
  
  id pool = [[NSAutoreleasePool alloc] init];

  invalidRect = NSMakeRect(0, 160, 172, 22);
  v = [[NSValue alloc]
  initWithBytes: &invalidRect
                 objCType: @encode(NSRect)];
   
   NSLog(@"value: %@", v);

  // The end...
  [pool release];

  return 0;
}

this test executes correctly, yiedling a valid value. So the issue is the context it is run in.

rmottola avatar Apr 17 '25 00:04 rmottola

It looks like the crash is because v is nil.

I think you could have a race condition on program exit ... if +[NSValue atExit:] is called before this code somehow, then the placeholder class will have been cleaned up and the code to create the NSValue instance will do nothing, so the instance will be nil.

You could confirm this by turning off the atExit: cleanup (comment out the call to +registerAtExit: in +initialize).

You could make the GUI code safe by having _setNeedsDisplayInRect_real: check whether its argument is nil.

rfm avatar Apr 17 '25 14:04 rfm

@rfm I too came the same conclusion about checking v, I did commit but you probably didn't see the branch, created a PR. I was unsure, since I thought it to be a workaround (albeit probably valid anyway, I think it hides another issue)

I also confirm your theory: disabling registerAtExit of NSValue makes the issue disappear too.

rmottola avatar Apr 17 '25 23:04 rmottola

@rfm I keep this bug open so we can hack on it, you mention in the merge request:

More fundamentally, we should probably remove the NSValue atExit code as it exists solely to avoid false positives with some memory leak checkers: I don't think asan or modern valgrind are subject to those false positives, and leak checkers generally support suppression lists to deal with false positives anyway.

The interesting thing would be to figure out how/why the NSView code is being called after the process exits. I suppose that might be due to some poor design of the way an app quits, but most likely it's really not important though.

In the meanwhile I will merge.

rmottola avatar Apr 24 '25 19:04 rmottola