Re: recent commit breaks multi-module debugging

Matthias Radestock <[email protected]> writes:

> Bill Clementson <[email protected]> writes:
>
>> The edb-monitor-cleanup function is called when the connection to the
>> node is lost.
>
> Good.
>
>>> When the edb buffer connects to the new node, it first received a list
>>> of interpreted modules. That should *clear* edb-interpreted-modules,
>>> since the node is new. Subsequently, toggling interpretation should just
>>> work. So how do we get this weird behaviour you are seeing?
>>
>> This appears to be the problem. I stepped through the code and noticed
>> that the edb-monitor-node variable wasn't being cleared by
>> edb-monitor-cleanup. Therefore, when I re-connect to the same node and
>> switch on interpreting, the edb-monitor-node-change-p function checks
>> to see whether the monitor buffer is live and whether the node has
>> changed:
>>
>> (defun edb-monitor-node-change-p (node)
>>   "Do we have to detach/reattach to debug on NODE?"
>>   (and (edb-monitor-live-p)
>>        (not (equal node edb-monitor-node))))
>>
>> Since the edb-monitor-node value is the same as the "new" node value,
>> Distel thinks that it's just working with the same node.
>
> Hang on. Notice the first branch of the 'and'? This function should
> always return false if there is no active monitor. Since, as you say
> above, the active monitor does indeed get killed when the node dies,
> that is the case we should be considering, in which case whether node
> equals edb-monitor-node makes no difference to the outcome.
>
> Now, edb-monitor-node-change-p returning false should actually be
> perfectly ok, because in that case edb-ensure-monitoring
> will run 
>     (if (edb-monitor-live-p)
> 	t
>       (edb-start-monitor node))))
> That will start a new monitor, which in turn will fetch the list of
> interpreted modules, breakpoints etc from the node.
>
> ...unless of course edb-monitor-live-p returns t. That really shouldn't
> happen if, as you say, the monitor buffer gets killed as expected when a
> node dies. Worth checking though. It would explain why your patch below
> makes a difference!

Ok, this is my take on what is happening (you'll have to follow along
in the edb.el code for this to make any sense):

The problem is that when a node goes down during debugging and the
interpreting state is "on", when you re-connect to the node, you have
to turn interpreting "off". So, the first time you press "C-c C-d i",
edb-toggle-interpret calls edb-ensure-monitoring which calls
edb-monitor-node-change-p which returns nil in the call to
edb-monitor-live-p because the edb monitor buffer has been
deleted. So, the "(if (edb-monitor-live-p)" returns nil and
edb-start-monitor is called. However, since the
interpreted/uninterpreted messages are out of sequence,
edb-interpreted-modules never gets set correctly. I'm not sure whether
this is because the erl-receive in the edb-start-monitor doesn't
receive an "interpreted" message and therefore the erlang node never
gets initialized correctly for the module and never sends the
&edb-monitor-loop function the correct message to set up the module in
the edb-interpreted-modules variable or whether it's somewhere
else. In any case, once edb-start-monitor is called "out-of-sequence",
you're basically hosed. So, when you subsequently toggle interactivity
"on", the monitor is up and the edb-monitor-node value is the same as
the "new" node value and the module never gets added to
edb-interpreted-modules.

By clearing out edb-monitor-node when the node goes down, the node is
treated (correctly) as a new node that hasn't been debugged previously
and edb-interpreted-modules gets updated correctly after the call to
edb-start-monitor.

Does that make sense? 

>> However, I'm surprised that this affects me but does not affect
>> you. When you went through my test case, did you re-connect with the
>> same node name or did you use a different node name?
>
> I used the same node name.

Which is confusing for me as I would have expected you to experience
the exact same problems that I was encountering.

>> Have a look at the attached patch (which removes my previous patch to
>> edb-toggle-interpret and clears the edb-monitor-node variable when
>> edb-monitor-cleanup is called.
>
> I am happy with this patch. I can think of no good reason why
> edb-monitor-node should be non-nil when there is no active
> monitor, and setting it to nil is probably a good idea, regardless of
> whether that is the cause of the problem you are experiencing.
>
>> Let me know if you see any potential problems with this patch.
>
> Feel free to commit it.
>
> However, I'd still like to know what the *real* cause of the problem
> is. It looks like in your setup edb-monitor-live-p is returning t when
> it shouldn't, which could easily cause problems elsewhere.

No, edb-monitor-live-p was working correctly. Hope my explanation
above made sense. I'll commit my patch. Thanks for all your help with
this.

Cheers,
Bill


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/