Re: recent commit breaks multi-module debugging

Matthias Radestock <[email protected]> Thu, 21 Jun 2007 08:41:29 +0100
Newsgroups gmane.comp.lang.erlang.distel.devel
Message-ID <[email protected]>
Bill,

Bill Clementson <[email protected]> writes:

>> For me, when the debugged node goes down, emacs kills the edb buffer and
>> resets the interpreted status of the modules I was debugging.
>>
>> Does that not happen for you?
>
> Yes, it kills the buffer; however, the module is left in a state of
> interpreted being "on"

Not for me.

When the edb buffer is killed it executes its kill-hook, which is
edb-monitor-cleanup:
  (setq edb-interpreted-modules '())
  (edb-delete-all-breakpoints)
  (edb-update-source-buffers)

Now, obviously there is a bug somewhere, otherwise you would not be
seeing the problem. From your explanation, it seems that perhaps killing
the edb buffer does not execute the above hook, at least not reliably.

> (if you followed the steps EXACTLY as they were
> in my test case, this is what should have happened after step
> #10.). Therefore, after I start up the node again, I have to toggle
> interpreted to "off" using "C-c C-d i".

It should already be off, since the node died, killing the edb buffer,
which in turn will have cleared edb-interpreted-modules and updated the
buffer statuses (see above).

My reading of the code is that you should only ever have modules in
an "interpreted" state when there is an edb buffer.

If that does not happen for you then we should try to figure out why.
It may be worth trying to call edb-monitor-cleanup manually at this
point resolves the problem.

> When I toggle it to "off", the
> edb-monitor-node-change-p function detects that the monitor buffer
> doesn't exist so it restarts it. Then, when I press "C-c C-d i" a
> second time to toggle interpreted "on", the edb buffer already
> exists. In this scenario, you can press "C-c C-d i" multiple times and
> it will not add the module to the edb-interpreted-modules
> variable.

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?

>> What's wrong with just killing the edb buffer? That should work; it has
>> a hook that clears edb-interpreted-modules, resets breakpoints etc.
>
> When you screw up the connection with a node while debugging, you are
> normally in a mode where interpreted is "on" in the module that you're
> debugging. Therefore, you would need to turn interpreted "off" and
> then delete the edb buffer in order to avoid the problem I described
> above. I would prefer to have this done automatically for me rather
> than have to remember to do it (and in the correct sequence) each
> time.

To re-iterate, in the original code, and from my experience:

* "screwing up the connection with a node" should result in the edb
buffer being killed which in turn, via its buffer kill-hook, should
clear the interpreted status of any modules.

* if, for whatever reason, you get into a weird state, killing the edb
buffer should restore normality.

> What specific problems do you feel there are with the revised patch
> that I suggested? It still allows you to do multi-module debugging
> across multiple nodes. You just have to toggle debugging on with "C-c
> C-d i" when you change the node that distel is connected to. You don't
> lose your breakpoints in any of the nodes that you're debugging.

My objection to the patch are as follows:

1) It does not fix the bug in the current code since we don't actually
know what the bug is, i.e. according to the current code the problem you
are seeing should not occur.

2) Mucking about with the edb state in erl-choose-nodename is
semantically wrong: erl-choose-nodename is meant to "Prompt the user for
the nodename to connect to *in future*.". Thus it should not perform any
action itself, other than setting the node name for future connections.
So, for example, setting the nodename to the same as it was before
should be a no-op. Also note that there is code in edb that detects node
name changes and prompts the user whether they want to connect to the
new node. If they decline, debugging continues in the old node. This
breaks with the patch.

3) The described problem does likely break things in many places.
Re-selecting the node in order to restore normality is not an obvious
action for a user to take.

4) There already exist commands/functions to clean up the state, namely
killing the edb buffer. If the edb buffer does not exist, manually
calling edb-monitor-cleanup should do the trick.


To move forward, we should figure out exactly what is going wrong for
you when a node dies:

Does the edb buffer not get killed?
or
Does the edb buffer get killed but edb-monitor-cleanup is not called?
or
Does the edb buffer get killed, edb-monitor-cleanup is called, but you
are still left in a weird state?


I'd also be happy for 4) to be wrapped up in a key-bound command
("edb-reset" or s.t. like that), since even if we fix the current
problem there may still be occasions when we get into a weird state
somehow, so it's good to have an escape hatch.


I appreciate the effort you have put into investigating this problem. I
want to get it fixed as much as you do, and am sure we can come up with
a solution that keeps everybody happy.


Regards,

Matthias.

-------------------------------------------------------------------------
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/