Re: tramp and diff-mode results in Emacs crash

Re: tramp and diff-mode results in Emacs crash

[Richard Stallman]

    - With Emacs and TRAMP both from Emacs' CVS, TRAMP had a problem that
      prevented me from attempting to reproduce the crash.

That problem is the bug we need to work on.  Have you sent a full report
for that bug?  If not, please do.

Once that is fixed, we will see if the other bug still exists.

Re: tramp and diff-mode results in Emacs crash

[Dale Sedivec]

On Sun, Feb 25, 2007 at 02:30:37PM -0500, Richard Stallman wrote:
>     - With Emacs and TRAMP both from Emacs' CVS, TRAMP had a problem that
>       prevented me from attempting to reproduce the crash.
> 
> That problem is the bug we need to work on.  Have you sent a full report
> for that bug?  If not, please do.
> 
> Once that is fixed, we will see if the other bug still exists.

	OK, I tried Emacs CVS checked out today, February 25th, at
16:25:45 EST, and TRAMP is working again.  I can now reproduce my bug
using the same procedure described previously:

- ~/software/emacs22/bin/emacs -q
- C-x C-f /10.0.1.169:foo.rej RET
- C-c C-u

	I see only "Back to top level."  The diff is unmodified by the
command.  Menus stop working, C-x C-b says "Back to top level." or
nothing, C-x b alternates between "S" and "Back to top level." in the
minibuffer without ever letting me change buffers, etc.

	So this bug exists in Emacs from CVS using the included TRAMP
2.0.55.

Dale

Re: tramp and diff-mode results in Emacs crash

[Michael Albinus]

Dale Sedivec <dale-dated-1172873680.43092c@codefu.org> writes:

> 	OK, I tried Emacs CVS checked out today, February 25th, at
> 16:25:45 EST, and TRAMP is working again.  I can now reproduce my bug
> using the same procedure described previously:
>
> - ~/software/emacs22/bin/emacs -q
> - C-x C-f /10.0.1.169:foo.rej RET
> - C-c C-u
>
> 	I see only "Back to top level."  The diff is unmodified by the
> command.  Menus stop working, C-x C-b says "Back to top level." or
> nothing, C-x b alternates between "S" and "Back to top level." in the
> minibuffer without ever letting me change buffers, etc.
>
> 	So this bug exists in Emacs from CVS using the included TRAMP
> 2.0.55.

What I can say is that I can reproduce the problem with a fresh GNU
Emacs 22.0.94 and builtin Tramp 2.0.55 (thanks for the example foo.rej).

With Tramp 2.1.9-pre from its CVS, same Emacs, this problem doesn't happen.

Hard to debug, because everything freezes. I suspect some concurrent
process filters fighting each other. I'll come back later (but this
might last one or two days).

> Dale

Best regards, Michael.

Re: tramp and diff-mode results in Emacs crash

[Chong Yidong]

Dale Sedivec <dale-dated-1172873680.43092c@codefu.org> writes:

> - ~/software/emacs22/bin/emacs -q
> - C-x C-f /10.0.1.169:foo.rej RET
> - C-c C-u
>
> 	I see only "Back to top level."  The diff is unmodified by the
> command.  Menus stop working, C-x C-b says "Back to top level." or
> nothing, C-x b alternates between "S" and "Back to top level." in the
> minibuffer without ever letting me change buffers, etc.

I checked in a fix for this.

(It's not a great fix, but it's the safest one I could think of.  The
problem arises when combine-after-change-calls is on.  The Tramp
filename handlers can be called (by lock_file) just before Emacs is
about to combine after-change calls, but those filename handlers can
themselves produce after-change calls because they scribble in temp
buffers.  It is possible to get Emacs into a confused state in this
way.  I simply made the Tramp handlers bind inhibit-modification-hooks
to avoid this problem.  We may want to revisit the interaction of file
modification hooks with combine-after-change-calls after the release.)

Re: tramp and diff-mode results in Emacs crash

[Stefan Monnier]

>> - ~/software/emacs22/bin/emacs -q
>> - C-x C-f /10.0.1.169:foo.rej RET
>> - C-c C-u
>> 
>> I see only "Back to top level."  The diff is unmodified by the
>> command.  Menus stop working, C-x C-b says "Back to top level." or
>> nothing, C-x b alternates between "S" and "Back to top level." in the
>> minibuffer without ever letting me change buffers, etc.

> I checked in a fix for this.

> (It's not a great fix, but it's the safest one I could think of.  The
> problem arises when combine-after-change-calls is on.  The Tramp
> filename handlers can be called (by lock_file) just before Emacs is
> about to combine after-change calls, but those filename handlers can
> themselves produce after-change calls because they scribble in temp
> buffers.  It is possible to get Emacs into a confused state in this
> way.  I simply made the Tramp handlers bind inhibit-modification-hooks
> to avoid this problem.  We may want to revisit the interaction of file
> modification hooks with combine-after-change-calls after the release.)

Maybe the workaround is OK, but I still don't understand what is the bug.
When using combine-after-change-calls, if changes are made in different
buffers, then the after-change-functions are run each time modifications are
done on a different buffer.

Oh, wait, you're saying that the problem is that when
combine-after-change-execute is run it begins by calling lock_file, which
causes more changes?  Hmm... I still don't see why that would be a problem.
Can you show some backtraces?


        Stefan

Re: tramp and diff-mode results in Emacs crash

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Oh, wait, you're saying that the problem is that when
> combine-after-change-execute is run it begins by calling lock_file, which
> causes more changes?  Hmm... I still don't see why that would be a problem.
> Can you show some backtraces?

I couldn't get Emacs to crash, but here is a backtrace from a
breakpoint just before Emacs gets confused.

Basically, insert_from_string calls prepare_to_modify_buffer before
signal_after_change (the latter is supposed to signal the changes due
to the insertion).  But prepare_to_modify_buffer calls lock_file,
which calls the Tramp file handlers, which does its own insertion in a
temp buffer, which runs signal_after_change, which first runs
signal_after_change on the original buffer---all this happens *before*
signal_before_change on the original buffer gets to run!

This analysis indicates that the more general solution is to bind
inhibit_modification_hooks around the call to lock_file in
prepare_to_modify_buffer, but that seems to me too risky a change at
this stage in the release.

#0  signal_after_change (charpos=1, lendel=0, lenins=12) at insdel.c:2257
#1  0x0811a625 in insert_from_string (string=141595915, pos=0, pos_byte=0, 
    length=12, length_byte=12, inherit=0) at insdel.c:1069
#2  0x08153f7d in general_insert_function (insert_func=0x811aab0 <insert>, 
    insert_from_string_func=0x811a5a0 <insert_from_string>, inherit=0, 
    nargs=1, args=0xbf89bd20) at editfns.c:2176
#3  0x081540b4 in Finsert (nargs=1, args=0xbf89bd20) at editfns.c:2220
#4  0x081874af in Fbyte_code (bytestr=141230795, vector=141367012, maxdepth=40)
    at bytecode.c:1258
#5  0x0815c2c7 in funcall_lambda (fun=141379860, nargs=2, 
    arg_vector=0xbf89be54) at eval.c:3184
#6  0x0815c711 in Ffuncall (nargs=3, args=0xbf89be50) at eval.c:3054
#7  0x0818793d in Fbyte_code (bytestr=141403235, vector=141259140, maxdepth=56)
    at bytecode.c:679
#8  0x0815c2c7 in funcall_lambda (fun=141142596, nargs=5, 
    arg_vector=0xbf89bf84) at eval.c:3184
#9  0x0815c711 in Ffuncall (nargs=6, args=0xbf89bf80) at eval.c:3054
#10 0x0818793d in Fbyte_code (bytestr=140040251, vector=139888212, maxdepth=80)
    at bytecode.c:679
#11 0x0815c2c7 in funcall_lambda (fun=140530676, nargs=2, 
    arg_vector=0xbf89c0c4) at eval.c:3184
#12 0x0815c711 in Ffuncall (nargs=3, args=0xbf89c0c0) at eval.c:3054
#13 0x0815e029 in Fapply (nargs=2, args=0xbf89c194) at eval.c:2485
#14 0x0815ca5a in Ffuncall (nargs=3, args=0xbf89c190) at eval.c:2978
#15 0x0818793d in Fbyte_code (bytestr=139878275, vector=141549748, maxdepth=32)
    at bytecode.c:679
#16 0x0815c2c7 in funcall_lambda (fun=141253452, nargs=3, 
    arg_vector=0xbf89c2b4) at eval.c:3184
#17 0x0815c711 in Ffuncall (nargs=4, args=0xbf89c2b0) at eval.c:3054
#18 0x0815e029 in Fapply (nargs=3, args=0xbf89c384) at eval.c:2485
#19 0x0815ca5a in Ffuncall (nargs=4, args=0xbf89c380) at eval.c:2978
#20 0x0818793d in Fbyte_code (bytestr=139878451, vector=141181604, maxdepth=32)
    at bytecode.c:679
#21 0x0815c2c7 in funcall_lambda (fun=139934292, nargs=3, 
    arg_vector=0xbf89c4a4) at eval.c:3184
#22 0x0815c711 in Ffuncall (nargs=4, args=0xbf89c4a0) at eval.c:3054
#23 0x0815cae9 in call3 (fn=138328817, arg1=137589513, arg2=141839179, 
    arg3=137472201) at eval.c:2827
#24 0x0812376c in Fexpand_file_name (name=141839179, 
    default_directory=137472201) at fileio.c:1721
#25 0x08117adc in lock_file (fn=141839179) at filelock.c:605
#26 0x0811869a in prepare_to_modify_buffer (start=1, end=1, preserve_ptr=0x0)
    at insdel.c:2085
#27 0x0811a2ec in insert_from_string_1 (string=<value optimized out>, pos=0, 
    pos_byte=0, nchars=3, nbytes=3, inherit=0, before_markers=0)
    at insdel.c:1121
#28 0x0811a607 in insert_from_string (string=139867003, pos=0, pos_byte=0, 
    length=3, length_byte=3, inherit=0) at insdel.c:1067
#29 0x08153f7d in general_insert_function (insert_func=0x811aab0 <insert>, 
    insert_from_string_func=0x811a5a0 <insert_from_string>, inherit=0, 
    nargs=1, args=0xbf89c660) at editfns.c:2176
#30 0x081540b4 in Finsert (nargs=1, args=0xbf89c660) at editfns.c:2220
#31 0x0815bf1b in Feval (form=138970861) at eval.c:2301
#32 0x0815c11f in Fprogn (args=137472201) at eval.c:447
#33 0x0815e755 in Flet (args=138970869) at eval.c:1064
#34 0x0815bf3d in Feval (form=138970901) at eval.c:2275
#35 0x0815c11f in Fprogn (args=137472201) at eval.c:447
#36 0x0815c3b4 in funcall_lambda (fun=138970784, nargs=0, 
    arg_vector=0xbf89c894) at eval.c:3177
#37 0x0815c711 in Ffuncall (nargs=1, args=0xbf89c890) at eval.c:3054
#38 0x0815e189 in apply1 (fn=138970789, arg=137472201) at eval.c:2738
#39 0x08159869 in Fcall_interactively (function=138970789, 
    record_flag=137472201, keys=137512716) at callint.c:406
#40 0x080f9313 in Fcommand_execute (cmd=138970789, record_flag=137472201, 
    keys=137472201, special=137472201) at keyboard.c:10014
#41 0x08104d39 in command_loop_1 () at keyboard.c:1873
#42 0x0815b2d2 in internal_condition_case (bfun=0x81049b0 <command_loop_1>, 
    handlers=137516857, hfun=0x80ff400 <cmd_error>) at eval.c:1481
#43 0x080fe7b3 in command_loop_2 () at keyboard.c:1329
---Type <return> to continue, or q <return> to quit---
#44 0x0815b38a in internal_catch (tag=137510841, 
    func=0x80fe790 <command_loop_2>, arg=137472201) at eval.c:1222
#45 0x080ff23c in command_loop () at keyboard.c:1308
#46 0x080ff5fa in recursive_edit_1 () at keyboard.c:1006
#47 0x080ff6e6 in Frecursive_edit () at keyboard.c:1067
#48 0x080f5405 in main (argc=3, argv=0xbf89d134) at emacs.c:1761

Lisp Backtrace:
"format-spec" (0x870950b)
"tramp-make-tramp-file-name" (0x831a8c9)
"tramp-handle-expand-file-name" (0x8744b4b)
"apply" (0x86fd461)
"tramp-sh-file-name-handler" (0x8337309)
"apply" (0x86fd9e1)
"tramp-file-name-handler" (0x8337309)
"insert" (0x856337b)
"let" (0x84886f5)
0x84886a5 Lisp type 5
"call-interactively" (0x84886a5)

Re: tramp and diff-mode results in Emacs crash

[Stefan Monnier]

>> Oh, wait, you're saying that the problem is that when
>> combine-after-change-execute is run it begins by calling lock_file, which
>> causes more changes?  Hmm... I still don't see why that would be a problem.
>> Can you show some backtraces?

> I couldn't get Emacs to crash, but here is a backtrace from a
> breakpoint just before Emacs gets confused.

> Basically, insert_from_string calls prepare_to_modify_buffer before
> signal_after_change (the latter is supposed to signal the changes due
> to the insertion).  But prepare_to_modify_buffer calls lock_file,
> which calls the Tramp file handlers, which does its own insertion in a
> temp buffer, which runs signal_after_change, which first runs
> signal_after_change on the original buffer---all this happens *before*
> signal_before_change on the original buffer gets to run!

Why is the above sequence a problem?

> This analysis indicates that the more general solution is to bind
> inhibit_modification_hooks around the call to lock_file in
> prepare_to_modify_buffer, but that seems to me too risky a change at
> this stage in the release.

It still feels like it'd hide the problem rather than fix it.
But maybe it's just because I still don't know what the problem is.


        Stefan

Re: tramp and diff-mode results in Emacs crash

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Basically, insert_from_string calls prepare_to_modify_buffer before
>> signal_after_change (the latter is supposed to signal the changes due
>> to the insertion).  But prepare_to_modify_buffer calls lock_file,
>> which calls the Tramp file handlers, which does its own insertion in a
>> temp buffer, which runs signal_after_change, which first runs
>> signal_after_change on the original buffer---all this happens *before*
>> signal_before_change on the original buffer gets to run!
>
> Why is the above sequence a problem?
>
> It still feels like it'd hide the problem rather than fix it.
> But maybe it's just because I still don't know what the problem is.

You're right, I wasn't thinking very clearly about the problem.  But I
now know what it is.

Basically, after the file handler performs its changes, it's time for
the original insertion to combine its after-change calls.  But it has
to perform combine-after-change execute for the temp file changes made
by the file handler.  But combine_after_change_buffer will no longer
be valid since the temp file was destroyed, and the call to set-buffer
in combine-after-change-execute fails.

So I reverted my previous change, and changed combine-after-change-execute
to return nil when combine_after_change_buffer is not a valid buffer.
This fixes the bug too, and I think it shouldn't be much more
dangerous than the previous workaround.

Re: tramp and diff-mode results in Emacs crash

[Stefan Monnier]

> So I reverted my previous change, and changed combine-after-change-execute
> to return nil when combine_after_change_buffer is not a valid buffer.
> This fixes the bug too, and I think it shouldn't be much more
> dangerous than the previous workaround.

That looks good, thank you.  The only problem that I can see with it, is
that we end up not running the hooks at all.  But there's no much we can do.
At best, we could try to run them before killing the buffer (e.g. from
Fkill_buffer) but I'm not sure it's worth the trouble.


        Stefan