Debugger bug: disappearing contents

On 2010/11/21 15:47, Frank Shearar wrote:

On 2010/11/13 17:35, Frank Shearar wrote:

Hi,

I've been chasing down a strange bug these past few days:
http://bugs.squeak.org/view.php?id=7569

The executive summary is this: when viewing a method in the debugger
that refers to an instvar deleted after the Debugger instantiated, the
CodePane shows the correct source for a fraction of a second, and then
goes blank.

What I'd expected to see was the source of the method rendered as per
usual, and the reference to the missing instvar coloured red, to
indicate a problem. (Just like what you'd see if you opened up a Browser
on the method.)

The CodePane goes blank because the Debugger's contents instvar is
nilled out.

CodeHolder>>setContentsToForceRefresh nils the instvar because
CodeHolder>>didCodeChangeElsewhere returns true.

That method returns true because the Debugger's currentCompiledMethod
instvar and the compiled method according to "aClass compiledMethodAt:
aSelector" aren't the same object.

And now I'm a bit stuck. I know the behaviour I'd expect, I know why the
bug's happening, but how do I fix it? Does CodeHolder (or Debugger) need
a better way of knowing whether the viewed method's changed? (For
instance, comparing the two CompiledMethods' asCommaString shows
identical contents.)

OK, I got a bit further. The Debugger's absolutely right in saying that
the code has changed elsewhere. As soon as you delete the instvar, the
entire class is recompiled and, in particular, the method referencing
the deleted instvar changes.

In my tests my method #bar looks like this:

bar
self halt.
baz := 1.

with bytecodes

17 <70> self
18 <D0> send: halt
19 <87> pop
20 <76> pushConstant: 1
21 <60> popIntoRcvr: 0
22 <78> returnSelf

After deleting the instvar baz, bar's bytecodes look like this:

21 <70> self
22 <D0> send: halt
23 <87> pop
24 <76> pushConstant: 1
25 <82 C1> popIntoLit: baz
27 <78> returnSelf

I can't argue with that. It does mean that the Debugger's job is harder.
In this particular case, nilling out the Debugger's contents is the
wrong thing to do, because it's surprising. You see the code, see the
reference to the defunct instvar... and then your source is gone!

We fall into this hole because for most CodeHolders, when the code's
changed, self hasUnacceptedEdits will return true, and you'll get that
red border around your code. In this case, there aren't unaccepted edits
because the user hasn't changed code. Instead, the Debugger has what I
suppose one might call a stale copy of the method (in contextStack).

So what should the correct behaviour be? How about a message that says
"Oops, this method has changed underneath your feet. Would you like to
revert back to the context that called it?"

I have something that works but, seeing as this is the Debugger, I'd appreciate someone else having a look.

It's in Tools-fbs.283, in the Inbox.

frank

On 2010/12/01 17:33, Eliot Miranda wrote:

On Wed, Dec 1, 2010 at 7:10 AM, Frank Shearar

<[hidden email] <mailto:[hidden email]>> wrote:

   On 2010/11/21 15:47, Frank Shearar wrote:

       On 2010/11/13 17:35, Frank Shearar wrote:

           Hi,

           I've been chasing down a strange bug these past few days:
           http://bugs.squeak.org/view.php?id=7569

           The executive summary is this: when viewing a method in the
           debugger
           that refers to an instvar deleted after the Debugger
           instantiated, the
           CodePane shows the correct source for a fraction of a
           second, and then
           goes blank.

           What I'd expected to see was the source of the method
           rendered as per
           usual, and the reference to the missing instvar coloured red, to
           indicate a problem. (Just like what you'd see if you opened
           up a Browser
           on the method.)

           The CodePane goes blank because the Debugger's contents
           instvar is
           nilled out.

           CodeHolder>>setContentsToForceRefresh nils the instvar because
           CodeHolder>>didCodeChangeElsewhere returns true.

           That method returns true because the Debugger's
           currentCompiledMethod
           instvar and the compiled method according to "aClass
           compiledMethodAt:
           aSelector" aren't the same object.

           And now I'm a bit stuck. I know the behaviour I'd expect, I
           know why the
           bug's happening, but how do I fix it? Does CodeHolder (or
           Debugger) need
           a better way of knowing whether the viewed method's changed?
           (For
           instance, comparing the two CompiledMethods' asCommaString shows
           identical contents.)


       OK, I got a bit further. The Debugger's absolutely right in
       saying that
       the code has changed elsewhere. As soon as you delete the
       instvar, the
       entire class is recompiled and, in particular, the method
       referencing
       the deleted instvar changes.

       In my tests my method #bar looks like this:

       bar
       self halt.
       baz := 1.

       with bytecodes

       17 <70> self
       18 <D0> send: halt
       19 <87> pop
       20 <76> pushConstant: 1
       21 <60> popIntoRcvr: 0
       22 <78> returnSelf

       After deleting the instvar baz, bar's bytecodes look like this:

       21 <70> self
       22 <D0> send: halt
       23 <87> pop
       24 <76> pushConstant: 1
       25 <82 C1> popIntoLit: baz
       27 <78> returnSelf

       I can't argue with that. It does mean that the Debugger's job is
       harder.
       In this particular case, nilling out the Debugger's contents is the
       wrong thing to do, because it's surprising. You see the code,
       see the
       reference to the defunct instvar... and then your source is gone!

       We fall into this hole because for most CodeHolders, when the code's
       changed, self hasUnacceptedEdits will return true, and you'll
       get that
       red border around your code. In this case, there aren't
       unaccepted edits
       because the user hasn't changed code. Instead, the Debugger has
       what I
       suppose one might call a stale copy of the method (in contextStack).

       So what should the correct behaviour be? How about a message
       that says
       "Oops, this method has changed underneath your feet. Would you
       like to
       revert back to the context that called it?"


   I have something that works but, seeing as this is the Debugger, I'd
   appreciate someone else having a look.

   It's in Tools-fbs.283, in the Inbox.


Looks good to me, but it might be wise to check that selectedContext is
not nil.  Theres' an assumption that contextStackTop is always valid and
I can imagine that changing.  Something like

contents
"Depending on the current selection, different information is retrieved.
Answer a string description of that information.  This information is the
method in the currently selected context."

^contents ifNil:
[self selectedContext
ifNotNil: [self selectedMessage]
ifNil: [String new]]

BTW, Personally I don't like the top context being shown when nothing is
selected in stack list  Is contextStackTop really necessary?

I wondered that myself, as I hacked on this.

With your change you will still see the top context when nothing's selected, at least once you have selected something. (But at first, if the debugger pops up and you hit "debug", then the code pane's blank.)

And you'll see that top context precisely because there's no concept of "nothing selected" - contextStackIndex = 0 means "show contextStackTop", and contextStackIndex > 0 means "you've selected something in the list of contexts".

Both your version and mine at least address http://bugs.squeak.org/view.php?id=7569

frank