bugGNUstep - Bugs: bug #22246, problem with key value observation

 
 

bug #22246: problem with key value observation

Submitted by:  Fred Kiefer <FredKiefer>
Submitted on:  Thu 07 Feb 2008 08:42:00 AM UTC  
 
Category:  Base/Foundation Severity:  3 - Normal
Item Group:  Bug Status:  Fixed
Privacy:  Public Assigned to:  Richard Frith-Macdonald <CaS>
Open/Closed:  Closed

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Mon 03 Mar 2008 10:50:59 PM UTC, comment #8: 

Great, my example works now perfectly.

Fred Kiefer <FredKiefer>
Project Member
Mon 03 Mar 2008 12:18:11 PM UTC, comment #7: 

Once I looked deep enough I spotted quite a few problems in the KVO code (as well as missing features), so in the end I rewrote the data structures and code for handling observations/notifications.  I hope this will fix everything, but as it's quite a major change it probably needs quite a bit of testing.

Richard Frith-Macdonald <CaS>
Project MemberIn charge of this item.
Sun 02 Mar 2008 12:30:07 PM UTC, comment #6: 

Your change reduced the amount of get calls from three to two. What happens shouldn't be too hard to understand. Instead of calling three overriden methods the code now calls two, setValue:forKey and the real setter method.

As we have two calls for the same key in one call stack, the behaviour that I diagnozed in my last entry still persists. The overriden code in setVale:forKey: sets up a change for this key, next it calls the original method, this calls the overriden setter, here again a change gets set up for the key (overriding the previous change for the same key). Then this method calls the original setter, next it gets the new value and sends the change notification. Finally it resets the change info to nil. The stack gets unrolled up to the overriden setValue:forKey:, here we try to send another change notification, but now the change info is nil, so all the values we get here are lost and nil is send as change.

What is a bit harder to say is, what the actual expected behaviour should look like. We should of course only send one change notification, but should that come from the overriden setter or from the overriden setValue:forKey method? I think Apple only uses the setter, but doing it in the setValue:forKey: method could be easier to implement. We would just check, whether there is already a change info stored for the current key and not process any notification stuff. That works nicely with any amount of overriden methods on the call stack.

Fred Kiefer <FredKiefer>
Project Member
Sun 02 Mar 2008 09:24:30 AM UTC, comment #5: 

You know I was having trouble at FOSDEM understanding the complicated sequence of methods that things were going through in the KVO code ... but couldn't really tell what I thought was wrong?

Well, I've figured it out ... the takeValue... stuff really shouldn't have been being used.

The KVC code is set up to call trhe obsolete/deprecated takeValue... methods from within the setValue... methods if the takeValue... methods have been overridden and are no longer the default NSObject ones.  The idea is that, if a programmer has overridden the old methods then they presumably want their implementations to be used.

The problem arises when the KVO code replaces the standard methods in order to monitor setting of ivars/properties.  The KVC code sees the overridden methods and assumes that it should call them.  I've changed the KVC code to recognise the case where the KVO code has overriden the deprecated methods, and not call them.

However, while this prevents the multiple calls to notify the observer when the value is set, it does not avoid the multiple calls to get the value ... leading me to suspect that our KVO implementation behavior is significantly different from that of Apple and we need to investigate and bring the behaviors more in line.

Richard Frith-Macdonald <CaS>
Project MemberIn charge of this item.
Sun 02 Mar 2008 05:17:43 AM UTC, comment #4: 

The diagnosis may well be correct, but the I simply can't be sure from looking at the example program.
The problem is that the test program is not the simple testcase for KVO I was looking for, but instead implicitly includes all the gui bindings stuff ... so without a clear understanding of bindings (and a surety that they are implemented the same way in macos/gnustep) it seems impossible to tell whether the difference in behaviors of the test app on the two systems is caused by a difference in KVO implementations or a difference in KVB implementations... That is to say, it's a test to demonstrate a gui bug without demonstrating whether the bug arises in the gui (KVB) or from an underlying fault in base (KVO).
I guess I'll have to look at the KVB code to see exactly what it does and produce a testcase for KVO.

Richard Frith-Macdonald <CaS>
Project MemberIn charge of this item.
Sat 01 Mar 2008 08:44:38 PM UTC, comment #3: 

I was able to work around the problem with the checkbox being switched off immediately. It turns out that this was caused by a call to observeValueForKeyPath:ofObject:change:context: with an empty change information.

This happens as the same change notification gets send on three levels by NSKeyValueObserving, after the first one is sent the change info is reset in didChangeValueForKey: now the next two levels find an empty change info, still they send it on. I would claim that this is a wrong behaviour and that we should just not send the notification, when the change has been already reset to empty.

Fred Kiefer <FredKiefer>
Project Member
Thu 28 Feb 2008 07:48:46 PM UTC, comment #2: 

Added a small test program to show the problem. This has a GNUmakefile as well as a XCode project file.

The behaviour is as described, when switching on the check box it switches off immediately. This happens only on GNUstep on Apple it behaves as expected.

(file #15136)

Fred Kiefer <FredKiefer>
Project Member
Tue 26 Feb 2008 11:22:39 AM UTC, comment #1: 

I think we need a simple testcase for this, so we can try it on MacOS-X and see what the expected behavior actually is.

Richard Frith-Macdonald <CaS>
Project MemberIn charge of this item.
Thu 07 Feb 2008 08:42:00 AM UTC, original submission:  

In gui I use KVO to implement KVB. The last change I made on that some time ago should allow a back chanel from the user inteface elements to the bound controller. Now this bit is failing. For a check box I get a strange effect. When I switch this checkbox on by clicking on it, it is shown on very shortly but gets switched off again immediatly. Debugging this showed that the call to setValue:For: on the controller results in three changes reported back to the user interface. One from the actual setSomething: method in the controller class (which is fine), one from takeValue:For: (which could be switched off, as far as I understand the code) and one from setValue:For:.
Now there should be a way that the KVO mechanism is able to tell that it already send the notification for a change and use the underlying methods of the base class to change the value, not the cooked up methods from the special class. But this isn't the real problem, what makes it worse for me is that the set value changes. This seems to happen in the takeValue:For: call, as I get the correct value once and then twice the wrong one.
It is this later behaviour that I don't understand. Anybody with an idea, what may be happening?

Fred Kiefer <FredKiefer>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #15136:  KVB_test.tgz added by FredKiefer (26KiB - application/x-tgz)

 

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 FredKiefer (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-03-03 FredKiefer Assigned toNone => CaS
        Open/ClosedIn Test => Closed
    2008-03-03 CaS Open/ClosedOpen => In Test
    2008-03-03 CaS StatusNone => Fixed
    2008-02-28 FredKiefer Attached File- => Added KVB_test.tgz, #15136

    Back to the top


    Powered by Savane 3.6