bugGNUstep - Bugs: bug #29338, Crash using @synchronized on...

Group
 
 

bug #29338: Crash using @synchronized on Windows

Submitter:  Doug Simons <theeggcamefirst>
Submitted:  Thu 25 Mar 2010 11:29:28 PM UTC
   
 
Category:  Base/Foundation Severity:  3 - Normal
Item Group:  Bug Status:  Fixed
Privacy:  Public Assigned to:  gcasa
Open/Closed:  In Test
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 19 Jan 2012 03:42:01 PM UTC, comment #9: 

It looks like this was fixed some time ago ... can anyone confirm?

Richard Frith-Macdonald <CaS>
Group Member
Mon 28 Nov 2011 05:40:39 AM UTC, comment #8: 

I believe the solution was to go back to using my slightly slower, but more stable implementation of these functions.

Gregory John Casamento <gcasa>
Group administrator
Sat 05 Mar 2011 04:42:02 PM UTC, comment #7: 

Re-opened this since,  as far as I know it was never fixed... unless perhaps I or someone else fixed it and I forgot.

Richard Frith-Macdonald <CaS>
Group Member
Fri 20 Aug 2010 09:56:41 PM UTC, comment #6: 

sync.m in libnobjc2 in svn trunk is broken at the latest revision:

It uses objc_disposeClassPair() ... which cannot work with the gnu runtime (ie can't be ported into ObjectiveC2 in base), and as far as I can see does not fully work in libobjc2 (though there we have enough internals access to fix it).

It doesn't align the mutex (which I recently found means that it will fail on hppa machines unless by chance the mutex falls on a 16 byte boundary).

Richard Frith-Macdonald <CaS>
Group Member
Fri 20 Aug 2010 08:59:20 PM UTC, comment #5: 

I thought David had fixed this in his most recent version of sync.m for the libobjc2 runtime.  This is acceptable for now, but I'll take a look into backporting David's changes into ObjectiveC2 under base.

Gregory John Casamento <gcasa>
Group administrator
Fri 20 Aug 2010 11:18:45 AM UTC, comment #4: 

I looked into it, and it seems that the new sync.m just can't work reliably/properly because it depends on unregistering classes (not supported by the underlying runtime).
The workaround would be to simply leak the classes and never unregister them ... probably not acceptable.
This code also incorrectly assumes that it has the mutex allocated in correctly aligned memory ... which is fixable if a little messy.

I replaced sync.m with Greg's original version which, while much slower, should actually work reliably.  In future, we could use a hashtable to improve performance somewhat if we want to.

I also modified the library to warn about bad/unsafe usage of objc_disposeClassPair() and objc_registerClassPair()

Richard Frith-Macdonald <CaS>
Group Member
Wed 16 Jun 2010 07:09:56 AM UTC, comment #3: 

Is this still waiting for Greg to backport the required changes, whatever they are, to objc_disposeClassPair()?

I see the current code still has the call to this commented out ... which is clearly a bug since it introduces a memory leak (not sure how major it is though ... the amount of memory leaked probably depends on how/when an app uses @synchronized).

It would be nice if this could be fixed before I make a 1.20.1 release.

Richard Frith-Macdonald <CaS>
Group Member
Mon 19 Apr 2010 10:37:14 PM UTC, comment #2: 

Has this problem been addressed?

Gregory John Casamento <gcasa>
Group administrator
Mon 29 Mar 2010 06:00:00 AM UTC, comment #1: 

Apparently there are some changes in libobjc2 which need backporting.  I'll do this unless someone else volunteers.

GC

Gregory John Casamento <gcasa>
Group administrator
Thu 25 Mar 2010 11:29:28 PM UTC, original submission:  

This only occurs when creating a lock for the second instance of our class, after the first instance has been deallocated.

Here's a backtrace that shows where it dies (stepping through code in sync.m):

78        class_addMethod(lockClass, @selector(dealloc), (IMP)deallocLockClass, types);
(gdb) n
80        if (!class_isMetaClass(obj->isa))
(gdb)
82            objc_registerClassPair(lockClass);
(gdb)

Program received signal SIGSEGV, Segmentation fault.
0x67844a2a in objc-1!__objc_resolve_class_links () from c:\GNUstep\GNUstep\System\Tools\objc-1.dll
(gdb) bt
#0  0x67844a2a in objc-1!__objc_resolve_class_links ()
   from c:\GNUstep\GNUstep\System\Tools\objc-1.dll
#1  0x65ac71f5 in objc_registerClassPair (cls=0x9dad9b0) at runtime.c:1002
#2  0x65ac7c7c in initLockObject (obj=0x99cbad0) at sync.m:82
#3  0x65ac7d9e in objc_sync_enter (obj=0x99cbad0) at sync.m:124
#4  0x00476425 in -[VNCConnection setStatusString:] (self=0x99cbad0, _cmd=0x54e6b0,
    aString=0x9b7e610) at VNCConnection.m:4449

I've committed a temporary fix for this, that simply removes the call to objc_disposeClassPair() in the deallocLockClass() function. Apparently the runtime can't handle disposing of a class and then creating a new class with the same superclass as the one that went away. I don't know if this is a Windows-specific problem or if it is an issue on Linux as well.

David, I understand from talking to Greg that you're the one who knows this stuff. If you have any questions about what's going on, give Greg a call.

Doug Simons <theeggcamefirst>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by CaS (Posted a comment)
  • -email is unavailable- added by gcasa (Posted a comment)
  • -email is unavailable- added by theeggcamefirst (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-01-19 CaS StatusIn Progress Fixed
        Open/ClosedOpen In Test
    2011-03-05 CaS StatusFixed In Progress
        Open/ClosedIn Test Open
    2010-08-20 CaS StatusNone Fixed
        Open/ClosedOpen In Test
    2010-03-29 gcasa Assigned toNone gcasa

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code