Thread: [Pyobjc-dev] Headache over hacking an XML editor
Brought to you by:
ronaldoussoren
From: Dinu G. <gh...@da...> - 2003-01-13 15:35:26
|
Hi, I'm banging my head against NSOutlineView's dataSource methods, trying to hack a little XML editor, and I'm not sure if it's just my sillyness or some bad surprises of PyObjC type conversion, sigh! In any case these methods don't seem to work as expected passing None as an item value far more often than I'd expect, so I wonder if anybody here has some simple code to share? I'd be glad to pro- vide the little test code I'm running myself... Dinu -- Dinu C. Gherman ...................................................................... "The pure and simple truth is rarely pure and never simple." (Oscar Wilde) |
From: Just v. R. <ju...@le...> - 2003-01-13 16:25:00
Attachments:
PythonBrowser.tgz
|
Dinu Gherman wrote: > Hi, I'm banging my head against NSOutlineView's dataSource methods, > trying to hack a little XML editor, and I'm not sure if it's just > my sillyness or some bad surprises of PyObjC type conversion, sigh! > > In any case these methods don't seem to work as expected passing > None as an item value far more often than I'd expect, so I wonder > if anybody here has some simple code to share? I'd be glad to pro- > vide the little test code I'm running myself... I've attached a little Python Object browser using an NSOutlineView I recently hacked up. The attached is slightly newer than what I posted here before. Just |
From: Just v. R. <ju...@le...> - 2003-01-13 20:27:53
|
I wrote: > I've attached a little Python Object browser using an NSOutlineView I > recently hacked up. The attached is slightly newer than what I posted > here before. Btw. there's in interesting thing going on in this app: if the PythonItem class is a subclass of NSObject everything works fine, but if it isn't (a pure Python class), the app crashes as soon as you click inside the tree widget. Maybe there's some refcount problem for Python objects passed to objc? Just |
From: Ronald O. <ous...@ci...> - 2003-01-13 21:46:27
|
On Monday, Jan 13, 2003, at 21:27 Europe/Amsterdam, Just van Rossum wrote: > I wrote: > >> I've attached a little Python Object browser using an NSOutlineView I >> recently hacked up. The attached is slightly newer than what I posted >> here before. > > Btw. there's in interesting thing going on in this app: if the > PythonItem class is a subclass of NSObject everything works fine, but > if > it isn't (a pure Python class), the app crashes as soon as you click > inside the tree widget. Maybe there's some refcount problem for Python > objects passed to objc? This is a feature(*) of NSOutlineView. The outline view does not call retain on the objects you return from methods like outlineView:child:ofItem:, but it does keep around references to those objects. This is documented in 'Using an Outline View Data Source' in Apple's developer documents. The reason your code is crashing when PythonItem is not subclassed from NSObject and runs correctly when it is twofold. First of all 'normal' python objects are brigded to Objective-C using autoreleased proxy objects. Because the outline-view doesn't call retain these will be released 'soon'. And because the outline-view does actually use the pointer later your program crashes... If you subclass from NSObject your object is bridged as itself (more or less). And if you keep around a reference to the objects you return these objects won't go away before the outline-view uses the pointer it stashed away. Which brings me to your second problem: You code is leaking memory. PythonItem.__new__ should return cls.alloc().init().release(). The call to release is necessary to adjust the reference count. The problem is that alloc is one of a few methods that returns a object that is owned by the caller, almost all other methods that return an object require that you call retain if you want to keep the object around. The bridge contains code to deal with this, but I didn't manage to convince Bill that automaticly adjusting the reference is a good idea. The mailinglist archive should contain more information. I still think that automaticly adjusting the references would be a good idea, and will get back to this later... Ronald (*) Actually I think this is not a feature but a bug. I've filed a bug for this at Apple, but did not receive a response for this yet. I hope Apple changes this, especially because adding the proper calls to retain and release should be backward compatible (although I have no idea what effect this would have on the performance of NSOutlineView) |
From: Just v. R. <ju...@le...> - 2003-01-13 22:11:15
|
Ronald Oussoren wrote: > On Monday, Jan 13, 2003, at 21:27 Europe/Amsterdam, Just van Rossum > wrote: > > > Btw. there's in interesting thing going on in this app: if the > > PythonItem class is a subclass of NSObject everything works fine, > > but if it isn't (a pure Python class), the app crashes as soon as > > you click inside the tree widget. Maybe there's some refcount > > problem for Python objects passed to objc? > > This is a feature(*) of NSOutlineView. The outline view does not call > retain on the objects you return from methods like > outlineView:child:ofItem:, but it does keep around references to > those objects. This is documented in 'Using an Outline View Data > Source' in Apple's developer documents. Aha, I'll try to find that document. > The reason your code is crashing when PythonItem is not subclassed > from NSObject and runs correctly when it is twofold. > > First of all 'normal' python objects are brigded to Objective-C using > autoreleased proxy objects. Because the outline-view doesn't call > retain these will be released 'soon'. And because the outline-view > does actually use the pointer later your program crashes... > > If you subclass from NSObject your object is bridged as itself (more > or less). And if you keep around a reference to the objects you > return these objects won't go away before the outline-view uses the > pointer it stashed away. Which brings me to your second problem: You > code is leaking memory. > > PythonItem.__new__ should return cls.alloc().init().release(). Ha, when I make that change the app crashes right away ;-) But the reason is now more clear: I didn't keep any child references around at all, so the instances NSObject subclass would be alive just long enough (due to the autorelease pool magic I assume) and the non-NSObject subclass instances weren't. Or they indeed leaked. I changed the code to keep references to the children and everything works just fine as a non-NSObject subclass. But now it *does* crash when subclassing NSObject if I add .release() to the __new__ method, so something isn't right, most likely in my understanding of things... > (*) Actually I think this is not a feature but a bug. I've filed a > bug for this at Apple, but did not receive a response for this yet. I > hope Apple changes this, especially because adding the proper calls > to retain and release should be backward compatible (although I have > no idea what effect this would have on the performance of > NSOutlineView) It's at least a lousy feature :-( Just |
From: Just v. R. <ju...@le...> - 2003-01-13 22:16:36
Attachments:
PythonBrowser.py
|
Btw. here's the version of PythonBrowser.py that keeps child references. Just |
From: Just v. R. <ju...@le...> - 2003-01-14 10:00:08
|
Just van Rossum wrote: > > PythonItem.__new__ should return cls.alloc().init().release(). Hm, I just discovered .release() returns None. If I do the release() separately it works just fine. I still don't understand why cls.alloc().init() would result in a retainCount of 2, though. Every time I see a retain() or release() call in Python code I can't help but think there's something terribly wrong :-( Just |
From: <bb...@ma...> - 2003-01-14 13:05:13
|
On Tuesday, Jan 14, 2003, at 05:00 US/Eastern, Just van Rossum wrote: >>> PythonItem.__new__ should return cls.alloc().init().release(). > > Hm, I just discovered .release() returns None. If I do the release() > separately it works just fine. I still don't understand why > cls.alloc().init() would result in a retainCount of 2, though. Every > time I see a retain() or release() call in Python code I can't help but > think there's something terribly wrong :-( It has to do with the way the retain/release mechanism works. In any case, you have one -retain for alloc() and one for the assignment in python. I agree that it would be very, very nice NOT to have to ever deal with retain/release/autorelease on the Python side of the PyObjC bridge. Unfortunately, it appears to be impossible [AFAICT] to automate the management of retain/release/autorelease with 100% reliability. Anything that makes assumptions regarding retain counts and adjusts the count downwards automatically will lead to situations where the app blows up. Ronald and I discussed this at length some time ago-- I believe it was before you had become active on the project. There is actually code to maintain the release counts automatically, it is currently commented out. Check the archives for more information.... b.bum |
From: Just v. R. <ju...@le...> - 2003-01-14 16:51:16
|
bb...@ma... wrote: > On Tuesday, Jan 14, 2003, at 05:00 US/Eastern, Just van Rossum wrote: > > Hm, I just discovered .release() returns None. If I do the > > release() separately it works just fine. I still don't understand > > why cls.alloc().init() would result in a retainCount of 2, though. > > Every time I see a retain() or release() call in Python code I > > can't help but think there's something terribly wrong :-( > > It has to do with the way the retain/release mechanism works. In > any case, you have one -retain for alloc() and one for the assignment > in python. Why? The caller of alloc() becomes the owner, and I don't see why the Python wrapper *wouldn't* become the owner. > I agree that it would be very, very nice NOT to have to ever deal > with retain/release/autorelease on the Python side of the PyObjC > bridge. Unfortunately, it appears to be impossible [AFAICT] to > automate the management of retain/release/autorelease with 100% > reliability. Anything that makes assumptions regarding retain counts > and adjusts the count downwards automatically will lead to situations > where the app blows up. I don't see why it's a bad "assumption" that the Python wrapper becomes the owner of the reference, IMHO it's only natural. I've tried to read the implementation (class-object.m) but I can't follow it at all because of the complexity. I seem to see that it attempts to synchronize the objc retainCount and the Python ob_refcnt. Why is that? I would think the Python wrapper for an objc object only needs to own one reference and vice versa for an objc wrapper for a Python object. I'm sure I'm missing something subtle, but now is as good a time to learn about how it works as any ;-) When creating a Python wrapper, there should be a difference between "wrap an existing ref" and "wrap a new ref". > Ronald and I discussed this at length some time ago-- I believe it > was before you had become active on the project. There is actually > code to maintain the release counts automatically, it is currently > commented out. Check the archives for more information.... I searched, to no avail. Any specific pointers or google keywords? Just |
From: <bb...@ma...> - 2003-01-14 17:12:58
|
On Tuesday, Jan 14, 2003, at 11:51 US/Eastern, Just van Rossum wrote: ... interesting stuff deleted ... > I searched, to no avail. Any specific pointers or google keywords? Basically, it boils down to the same reason why we don't do name mangling -- it is easy to do something that works 90% of the time, impossible to do something that works 100% of the time and that 10% of the time is a real serious pain in the butt to debug/deal with. When using PyObjC, you cannot ignore the fact that you are effectively using Objective-C from Python. The memory management semantics of ObjC *will* leak through the bridge due to inconsistencies and idiosyncracies within the implementation of classes -- known and unknown (i.e. third party) -- on the ObjC side. Thread starts here: http://sourceforge.net/mailarchive/message.php?msg_id=2347617 But this particular response [of mine] contains the most detailed explanation on why I don't believe it is possible to eliminate retain/release/autorelease entirely from the Python side of the bridge: http://sourceforge.net/mailarchive/message.php?msg_id=2349810 And another thread that may also be illuminating: http://sourceforge.net/mailarchive/message.php?msg_id=2342178 And another thread: http://sourceforge.net/mailarchive/message.php?msg_id=2331515 b.bum |
From: Just v. R. <ju...@le...> - 2003-01-14 18:57:28
|
bb...@ma... wrote: > Thread starts here: > > http://sourceforge.net/mailarchive/message.php?msg_id=2347617 > > But this particular response [of mine] contains the most detailed > explanation on why I don't believe it is possible to eliminate > retain/release/autorelease entirely from the Python side of the > bridge: > > http://sourceforge.net/mailarchive/message.php?msg_id=2349810 > > And another thread that may also be illuminating: > > http://sourceforge.net/mailarchive/message.php?msg_id=2342178 > > And another thread: > > http://sourceforge.net/mailarchive/message.php?msg_id=2331515 Ugh, this is depressing :-(. (So you _can_ find out the return type of a method at runtime but not whether it returns a new or a borrowed reference. Great.) Here's a quick idea to work around the most common (I think) irritation: instead of raising an exception when calling a (subclass of) NSObject ("TypeError: Use class methods to instantiate new Objective-C objects") have objc-object.m::object_new() do this: self = cls.alloc() self.release() # ownership transferred to the Python wrapper return self This doesn't touch the alloc() semantics, yet provides a much more Pythonic way to create instances. Heck, this will even call __init__() for you! ;-) What I still don't understand is how pyobjc manages the refcount of Python objects passed to objc: the Python object can't have a reference to it, so when will the wrapper be released? Is it an autorelease object? Just |
From: <bb...@ma...> - 2003-01-14 19:12:44
|
On Tuesday, Jan 14, 2003, at 13:57 US/Eastern, Just van Rossum wrote: > Ugh, this is depressing :-(. (So you _can_ find out the return type of > a > method at runtime but not whether it returns a new or a borrowed > reference. Great.) Correct. There is no concept of 'new vs. borrowed' at anything but the object level. This isn't a problem from the Objective-C side of the fence -- it is only a problem in that Python and ObjC take different approaches to solving the memory management problem. I like Python's a lot better, but I have to live with ObjC's because I'm leveraging ObjC frameworks-- even when writing PyObJC code. > Here's a quick idea to work around the most common (I think) > irritation: > instead of raising an exception when calling a (subclass of) NSObject > ("TypeError: Use class methods to instantiate new Objective-C objects") > have objc-object.m::object_new() do this: > > self = cls.alloc() > self.release() # ownership transferred to the Python wrapper > return self This code is lacking the call to the -init method -- problematic. What about arguments to the constructor? We can handle the no argument case transparently, but what about-- say-- NSView's -initWithFrame: designated initializer? Also, keep in mind that -init* methods return an object reference for a reason; a number of intializers return a different instance than the one that was called for a number of [valid] reasons. > This doesn't touch the alloc() semantics, yet provides a much more > Pythonic way to create instances. Heck, this will even call __init__() > for you! ;-) But __init__() is not generally the designated intiailizer for ObjC classes-- even Python implemented subclasses of ObjC classes. When subclassing ObjC from Python, it makes more sense to follow the ObjC semantics than the Python semantics -- at least, it does to me. > What I still don't understand is how pyobjc manages the refcount of > Python objects passed to objc: the Python object can't have a reference > to it, so when will the wrapper be released? Is it an autorelease > object? Ronald can better answer this question. b.bum |
From: Just v. R. <ju...@le...> - 2003-01-14 19:24:04
|
bb...@ma... wrote: > > self = cls.alloc() > > self.release() # ownership transferred to the Python wrapper > > return self > > This code is lacking the call to the -init method -- problematic. I was thinking of this pattern, which is not 100% Pythonic, yet 100% better than having to do a release() "by hand": inst = ASubclassOfNSObject().init() So basically "i = cls()" is equivalent to "i = cls.alloc(); i.release()". > What about arguments to the constructor? We can handle the no > argument case transparently, but what about-- say-- NSView's > -initWithFrame: designated initializer? aView = NSView().initWithFrame(...) > Also, keep in mind that -init* methods return an object reference for > a reason; a number of intializers return a different instance than > the one that was called for a number of [valid] reasons. Solved above ;-) > > This doesn't touch the alloc() semantics, yet provides a much more > > Pythonic way to create instances. Heck, this will even call > > __init__() for you! ;-) > > But __init__() is not generally the designated intiailizer for ObjC > classes-- even Python implemented subclasses of ObjC classes. > > When subclassing ObjC from Python, it makes more sense to follow the > ObjC semantics than the Python semantics -- at least, it does to me. Sure, but calling __init__ won't hurt either if you're writing an NSObject subclass solely to be instantiated from Python. But I'm afraid this behavior is implicit in the Python object protocol (since 2.2). I _think_ it's this: inst = cls.__new__(cls, *args, **kwargs) if isinstance(inst, cls): inst.__init__(*args, **kwargs) (I also think it's identical for tp_new/tp_init on the C side.) Just |
From: <bb...@ma...> - 2003-01-14 19:39:54
|
On Tuesday, Jan 14, 2003, at 14:24 US/Eastern, Just van Rossum wrote: > bb...@ma... wrote: > >>> self = cls.alloc() >>> self.release() # ownership transferred to the Python > wrapper >>> return self >> >> This code is lacking the call to the -init method -- problematic. > > I was thinking of this pattern, which is not 100% Pythonic, yet 100% > better than having to do a release() "by hand": > > inst = ASubclassOfNSObject().init() > > So basically "i = cls()" is equivalent to "i = cls.alloc(); > i.release()". There are cases where this will not work -- Ronald has already had to special case around a number of them. NSData, NSArray, and NSCell (I think) are examples. Not because of bugs in Foundation/AppKit, but because of the implementation pattern used. It would have to be something like.... i = cls.alloc() i.autorelease() ... anyway. (but won't work because of the reasons noted above) >> What about arguments to the constructor? We can handle the no >> argument case transparently, but what about-- say-- NSView's >> -initWithFrame: designated initializer? > > aView = NSView().initWithFrame(...) Makes sense. >> Also, keep in mind that -init* methods return an object reference for >> a reason; a number of intializers return a different instance than >> the one that was called for a number of [valid] reasons. > > Solved above ;-) Except manipulating the retain count on the return value from +alloc* leads to really nasty crashes (of which, some *are* bugs in the Foundation/AppKit -- Ronald has reported a couple, I believe). >>> This doesn't touch the alloc() semantics, yet provides a much more >>> Pythonic way to create instances. Heck, this will even call >>> __init__() for you! ;-) >> >> But __init__() is not generally the designated intiailizer for ObjC >> classes-- even Python implemented subclasses of ObjC classes. >> >> When subclassing ObjC from Python, it makes more sense to follow the >> ObjC semantics than the Python semantics -- at least, it does to me. > > Sure, but calling __init__ won't hurt either if you're writing an > NSObject subclass solely to be instantiated from Python. But I'm afraid > this behavior is implicit in the Python object protocol (since 2.2). I > _think_ it's this: > > inst = cls.__new__(cls, *args, **kwargs) > if isinstance(inst, cls): > inst.__init__(*args, **kwargs) > > (I also think it's identical for tp_new/tp_init on the C side.) True. And I'm all for anything that can make the Python side of the bridge more pythonic -- as long as it can be done consistently and without introducing instability. b.bum |
From: Just v. R. <ju...@le...> - 2003-01-14 19:49:44
|
bb...@ma... wrote: > There are cases where this will not work -- Ronald has already had to > special case around a number of them. NSData, NSArray, and NSCell (I > think) are examples. Not because of bugs in Foundation/AppKit, but > because of the implementation pattern used. So how _do_ you instantiate these classes? Just |
From: <bb...@ma...> - 2003-01-14 19:55:30
|
On Tuesday, Jan 14, 2003, at 14:46 US/Eastern, Just van Rossum wrote: >> There are cases where this will not work -- Ronald has already had to >> special case around a number of them. NSData, NSArray, and NSCell (I >> think) are examples. Not because of bugs in Foundation/AppKit, but >> because of the implementation pattern used. > > So how _do_ you instantiate these classes? Very carefully. :-) Have a look at the alloc hack stuff in the source... b.bum |
From: Ronald O. <ous...@ci...> - 2003-01-14 22:15:21
|
On Tuesday, Jan 14, 2003, at 20:55 Europe/Amsterdam, bb...@ma... wrote: > On Tuesday, Jan 14, 2003, at 14:46 US/Eastern, Just van Rossum wrote: >>> There are cases where this will not work -- Ronald has already had to >>> special case around a number of them. NSData, NSArray, and NSCell >>> (I >>> think) are examples. Not because of bugs in Foundation/AppKit, but >>> because of the implementation pattern used. >> >> So how _do_ you instantiate these classes? > > Very carefully. :-) > > Have a look at the alloc hack stuff in the source... The alloc-hack stuff is for another bug/feature of Cocoa. Some classes don't like calls to +alloc through an NSInvocation. I've opened a bugreport for this with Apple, but so far without result. You have to be a bit carefull: call alloc, some variation of init and then adjust the reference count. Doing it in another order will cause problems some times, mostly when using class clusters. Ronald |
From: Ronald O. <ous...@ci...> - 2003-01-14 22:32:38
|
On Tuesday, Jan 14, 2003, at 20:39 Europe/Amsterdam, bb...@ma... wrote: > On Tuesday, Jan 14, 2003, at 14:24 US/Eastern, Just van Rossum wrote: >> bb...@ma... wrote: >> >>>> self = cls.alloc() >>>> self.release() # ownership transferred to the Python >> wrapper >>>> return self >>> >>> This code is lacking the call to the -init method -- problematic. >> >> I was thinking of this pattern, which is not 100% Pythonic, yet 100% >> better than having to do a release() "by hand": >> >> inst = ASubclassOfNSObject().init() >> >> So basically "i = cls()" is equivalent to "i = cls.alloc(); >> i.release()". > > There are cases where this will not work -- Ronald has already had to > special case around a number of them. NSData, NSArray, and NSCell (I > think) are examples. Not because of bugs in Foundation/AppKit, but > because of the implementation pattern used. > > It would have to be something like.... > > i = cls.alloc() > i.autorelease() No, it is i.release(). the cls.alloc() returns an object where we own the reference, and then the bridge calls retain. We therefore have to call release to get the correct reference-count. I still think we can get this working >95% of the time, with the other <5% being methods that also return a new reference but that we don't know of. The current code to deal with this is slightly to simpleminded, but IMHO it is possible to correctly recognize an 'alloc + init' sequence and then adjust the reference count. The other documented cases of method returning a new reference can be processed by the current logic. The Apple Objective-C manual mentions about 5 methods that return new references, all others should return borrowed references. As Bill noted earlier real code doesn't always follow these conventions, but I think it would be sick to return a borrowed reference from an 'alloc + init' sequence. I hope to write down a design for this soonish, but I'd like to do some performance testing before that. > > ... anyway. (but won't work because of the reasons noted above) > >>> What about arguments to the constructor? We can handle the no >>> argument case transparently, but what about-- say-- NSView's >>> -initWithFrame: designated initializer? >> >> aView = NSView().initWithFrame(...) > > Makes sense. But als you note below not really feasable unless we'd use 'def __new__(cls): return cls.alloc()'. Hmm, I kind of like this, but we should document that you *must* call an init-method afterwards. The following objective-C code might crash: [[SomeClass alloc] release]. This does crash for some classes in Cocoa and according to Apple this is correct behaviour: Until you call an init method the instance is not instantiated and therefore any method-call might fail. > >>> Also, keep in mind that -init* methods return an object reference for >>> a reason; a number of intializers return a different instance than >>> the one that was called for a number of [valid] reasons. >> >> Solved above ;-) > > Except manipulating the retain count on the return value from +alloc* > leads to really nasty crashes (of which, some *are* bugs in the > Foundation/AppKit -- Ronald has reported a couple, I believe). According to Apple these are not bugs, you should only call an init method on the reference returned by +alloc. Me thinks this is a sad situation :-(. > >>>> This doesn't touch the alloc() semantics, yet provides a much more >>>> Pythonic way to create instances. Heck, this will even call >>>> __init__() for you! ;-) >>> >>> But __init__() is not generally the designated intiailizer for ObjC >>> classes-- even Python implemented subclasses of ObjC classes. >>> >>> When subclassing ObjC from Python, it makes more sense to follow the >>> ObjC semantics than the Python semantics -- at least, it does to me. >> >> Sure, but calling __init__ won't hurt either if you're writing an >> NSObject subclass solely to be instantiated from Python. But I'm >> afraid >> this behavior is implicit in the Python object protocol (since 2.2). I >> _think_ it's this: >> >> inst = cls.__new__(cls, *args, **kwargs) >> if isinstance(inst, cls): >> inst.__init__(*args, **kwargs) >> >> (I also think it's identical for tp_new/tp_init on the C side.) > > True. And I'm all for anything that can make the Python side of the > bridge more pythonic -- as long as it can be done consistently and > without introducing instability. If adding a __new__ method would cause __init__ to be called when the class is instantiated from Python, we should make sure that __init__ is also called when the class is instantiated from Objective-C, otherwise we'd get some very odd behaviour ('It works when I run my test script, but doesn't when I use it from Interface Builder'). Ronald |
From: Dinu G. <gh...@da...> - 2003-01-14 13:07:22
|
Just van Rossum: > I've attached a little Python Object browser using an NSOutlineView I > recently hacked up. The attached is slightly newer than what I posted > here before. Thanks, this looks very promising and useful for a future Cocoa Python IDE! ;-) Ronald, thanks for your comemnts as well! Moving to dedicated item class subclassed from NSObject finally did the job! This is a current state screenshot: http://python.net/~gherman/tmp/XMLTreeView1.png Now, I'll need to move from displaying to editing XML, possibly with auto-validation as you go... Dinu -- Dinu C. Gherman ...................................................................... "I can't understand why people are frightened of new ideas. I'm frightened of the old ones." (John Cage) |