Pretest?

Pretest?

[Juanma Barranquero]

A month since 22.0.93, and the flow of patches has slowed down quite a
bit. Perhaps we should issue another pretest while we wait for the
legal issues to be sorted out?

             Juanma

Re: Pretest?

[Eli Zaretskii]

> Date: Fri, 23 Feb 2007 10:30:31 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> 
> A month since 22.0.93, and the flow of patches has slowed down quite a
> bit. Perhaps we should issue another pretest while we wait for the
> legal issues to be sorted out?

I'd prefer if we waited for a couple more days.  I have a few
important patches in my sandbox, and I'd like to see whether the
latest changes in w32menu.c don't cause any trouble elsewhere.

Re: Pretest?

[Chong Yidong]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> A month since 22.0.93, and the flow of patches has slowed down quite a
> bit. Perhaps we should issue another pretest while we wait for the
> legal issues to be sorted out?

I agree.  (I was hoping Alan's C++ misfontification fix could come in
time, but it can wait for the next tarball.)

I have rolled a 22.0.94 tarball:

ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.94.tar.gz
ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.93-22.0.94.xdelta.sig

(I didn't see Eli's message till I uploaded the tarball.  Hopefully
those patches can wait for the next tarball, along with the PGG doc
fixes and the C++ fontification fix.)

Re: Pretest?

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Fri, 23 Feb 2007 13:48:09 -0500
> Cc: Emacs-Devel <emacs-devel@gnu.org>
> 
> I have rolled a 22.0.94 tarball:
> 
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.94.tar.gz
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.93-22.0.94.xdelta.sig
> 
> (I didn't see Eli's message till I uploaded the tarball.  Hopefully
> those patches can wait for the next tarball, along with the PGG doc
> fixes and the C++ fontification fix.)

Please in the future wait for a while before you act.  I don't always
have time to react immediately, and I imagine others have such
problems as well.  My free time is very limited, so I usually can work
on Emacs only on weekends.  Thus, any changes that I need to make that
take more than a few moments are delayed by several days.

I suggest in the future to announce that you will produce the next
pretest on such-and-such date (several days in the future), so that
people who have important patches could install them in time for the
pretest.

Re: Pretest?

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> Please in the future wait for a while before you act.  I don't always
> have time to react immediately, and I imagine others have such
> problems as well.  My free time is very limited, so I usually can work
> on Emacs only on weekends.  Thus, any changes that I need to make that
> take more than a few moments are delayed by several days.
> 
> I suggest in the future to announce that you will produce the next
> pretest on such-and-such date (several days in the future), so that
> people who have important patches could install them in time for the
> pretest.

Yes, you're right.

Maybe we should avoid announcing this pretest, and roll the next
pretest in short order, so that it can include the GPG doc changes,
the patches you are testing, and (hopefully) Alan's C++ fix.  How
about a week, say the weekend of March 1?

Re: Pretest?

[Eli Zaretskii]

> Cc: lekktu@gmail.com, emacs-devel@gnu.org
> From: Chong Yidong <cyd@MIT.EDU>
> Date: 24 Feb 2007 10:40:49 -0500
> 
> Maybe we should avoid announcing this pretest, and roll the next
> pretest in short order, so that it can include the GPG doc changes,
> the patches you are testing, and (hopefully) Alan's C++ fix.  How
> about a week, say the weekend of March 1?

That'd be fine with me, thanks.  But let's wait for Richard to comment
on that.

Re: Pretest?

[Richard Stallman]

I think a new pretest March 1 would be good.

Re: Pretest?

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> I think a new pretest March 1 would be good.

OK, March 1 it is.

Re: Pretest?

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> I think a new pretest March 1 would be good.

I have rolled a 22.0.95 tarball, which can be found at the usual
location:

ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.95.tar.gz
ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.94-22.0.95.xdelta

Personally, I think we should plan to release soon (assuming we can
resolve or work around the remaining C++ fontification bug).

Re: Pretest?

[Lennart Borgman (gmail)]

Chong Yidong wrote:
> Richard Stallman <rms@gnu.org> writes:
> 
>> I think a new pretest March 1 would be good.
> 
> I have rolled a 22.0.95 tarball, which can be found at the usual
> location:
> 
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.95.tar.gz
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.94-22.0.95.xdelta
> 
> Personally, I think we should plan to release soon (assuming we can
> resolve or work around the remaining C++ fontification bug).


New precompiled binaries (unpatched) available at

   http://ourcomments.org/cgi-bin/emacsw32-dl-latest.pl

Re: Pretest?

[Juanma Barranquero]

On 3/2/07, Chong Yidong <cyd@stupidchicken.com> wrote:

> Personally, I think we should plan to release soon (assuming we can
> resolve or work around the remaining C++ fontification bug).

I was under the impression our waiting was mainly caused by lawyers, not bugs.

Now that I think of it... Hmm...

             Juanma

Re: Pretest?

[David Kastrup]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/2/07, Chong Yidong <cyd@stupidchicken.com> wrote:
>
>> Personally, I think we should plan to release soon (assuming we can
>> resolve or work around the remaining C++ fontification bug).
>
> I was under the impression our waiting was mainly caused by lawyers,
> not bugs.
>
> Now that I think of it... Hmm...

Gregor Samsa was a travelling salesman.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Pretest?

[Juanma Barranquero]

On 3/2/07, David Kastrup <dak@gnu.org> wrote:

> Gregor Samsa was a travelling salesman.

Now *that*'s a beatiful thought when dealing with some salesmen I've met...

             Juanma

Re: Pretest?

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Thu, 01 Mar 2007 18:38:43 -0500
> 
> I have rolled a 22.0.95 tarball, which can be found at the usual
> location:
> 
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.95.tar.gz
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.94-22.0.95.xdelta

I tested this with:

   . GCC 3.4.2 (mingw-special), Binutils 2.15.91 20040904 and MinGW
     v3.7 on Windows XP

   . GCC 4.0.3 (Ubuntu 4.0.3-1ubuntu5), Binutils 2.16.91 20060118 on
     Linux fencepost 2.6.16.29-xen #1 SMP Wed Dec 6 07:32:36 EST 2006
     x86_64 GNU/Linux

The pretest builds okay on both platforms and appears to run fine.

Re: Pretest?

[Giorgos Keramidas]

On 2007-03-01 18:38, Chong Yidong <cyd@stupidchicken.com> wrote:
>Richard Stallman <rms@gnu.org> writes:
>> I think a new pretest March 1 would be good.
>
> I have rolled a 22.0.95 tarball, which can be found at the usual
> location:
>
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.95.tar.gz
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.94-22.0.95.xdelta

I just finished build-testing on:

  FreeBSD 7.0-CURRENT #0: Tue Feb 27 01:25:46 EET 2007

This pretest builds fine with both Lucid widgets and GTK+ widgets.

The Lucid version runs fine (it's what I've been using since last
October, when I switched to Emacs 22.X).

While I'm running the GTK+ version, however, I can crash Emacs in
emacs_blocked_free() by following the steps outlined below:

* Run Emacs inside gdb:

,-----------------------------------------------------------------------
|
| keramida@kobe:/home/keramida/tmp/emacs-22.0.95/src$ gdb ./emacs-22.0.95.1
| GNU gdb 6.1.1 [FreeBSD]
| Copyright 2004 Free Software Foundation, Inc.
| GDB is free software, covered by the GNU General Public License, and you are
| welcome to change it and/or distribute copies of it under certain conditions.
| Type "show copying" to see the conditions.
| There is absolutely no warranty for GDB.  Type "show warranty" for details.
| This GDB was configured as "i386-marcel-freebsd"...No symbol table is loaded.  Use the "file" command.
|
| DISPLAY = :0
| TERM = vt220
| Breakpoint 1 at 0x80e7c0a: file emacs.c, line 431.
| Breakpoint 2 at 0x80ff6fd: file sysdep.c, line 1385.
| (gdb) r
| Starting program: /home/keramida/tmp/emacs-22.0.95/src/emacs-22.0.95.1 -geometry 80x40+0+0
| warning: Unable to get location for thread creation breakpoint: generic error
| [New LWP 100067]
| [New Thread 0x8424800 (LWP 100067)]
| [Switching to Thread 0x8424800 (LWP 100067)]
| Breakpoint 3 at 0x80c6fcc: file xterm.c, line 7852.
| [New Thread 0x8424a00 (LWP 100205)]
|
| [...]
`-----------------------------------------------------------------------

* Run M-x gnus-agent-batch while my network connection is
  disabled, and let it time-out.  It prompts me for going into
  `off-line mode', to which I reply `yes'.

* The next time I input C-z Emacs crashes with a backtrace of:

,-----------------------------------------------------------------------
|
| Program received signal SIGSEGV, Segmentation fault.
| 0x081895e0 in _free_internal (ptr=0x29a82300) at gmalloc.c:1197
| 1197              next->next = prev->next;
| (gdb) bt
| #0  0x081895e0 in _free_internal (ptr=0x29a82300) at gmalloc.c:1197
| #1  0x08133683 in emacs_blocked_free (ptr=0x29a82300, ptr2=0xbfbfdbf4) at alloc.c:1207
| #2  0x288694c4 in g_slice_get_config_state () from /usr/local/lib/libglib-2.0.so.0
| #3  0x28869713 in g_slice_get_config_state () from /usr/local/lib/libglib-2.0.so.0
| #4  0x28869887 in g_slice_get_config_state () from /usr/local/lib/libglib-2.0.so.0
| #5  0x28869f5c in g_slice_free1 () from /usr/local/lib/libglib-2.0.so.0
| #6  0x2884a41b in g_hash_table_ref () from /usr/local/lib/libglib-2.0.so.0
| #7  0x2884acf4 in g_hash_table_remove_all () from /usr/local/lib/libglib-2.0.so.0
| #8  0x2884ad8c in g_hash_table_destroy () from /usr/local/lib/libglib-2.0.so.0
| #9  0x298e8c7a in pixbuf_create_from_xpm () from /usr/local/lib/gtk-2.0/2.10.0/loaders/libpixbufloader-xpm.so
| #10 0x298e92ba in gdk_pixbuf__xpm_image_load_xpm_data ()
|    from /usr/local/lib/gtk-2.0/2.10.0/loaders/libpixbufloader-xpm.so
| #11 0x28583024 in gdk_pixbuf_new_from_xpm_data () from /usr/local/lib/libgdk_pixbuf-2.0.so.0
| #12 0x080c8f87 in xg_set_icon_from_xpm_data (f=0x7f, data=0x81a0e60) at xfns.c:830
| #13 0x080c4d97 in x_bitmap_icon (f=0x83b0e00, file=137435185) at xterm.c:7444
| #14 0x080c4ea4 in x_iconify_frame (f=0x83b0e00) at xterm.c:9162
| #15 0x0805c92b in Ficonify_frame (frame=1645188) at frame.c:1711
| #16 0x08148cab in Ffuncall (nargs=1, args=0x819f2c8) at eval.c:3000
| #17 0x08170a59 in Fbyte_code (bytestr=1645188, vector=-1077943888, maxdepth=0) at bytecode.c:679
| #18 0x0814872f in funcall_lambda (fun=136384076, nargs=0, arg_vector=0xbfbfe304) at eval.c:3184
| #19 0x08148b5a in Ffuncall (nargs=1, args=0x8210e4c) at eval.c:3054
| #20 0x0814a0d2 in apply1 (fn=139454561, arg=137435137) at eval.c:2738
| #21 0x081463fc in Fcall_interactively (function=139454561, record_flag=137435137, keys=137363204) at callint.c:406
| #22 0x080ef01d in Fcommand_execute (cmd=139454561, record_flag=137435137, keys=137435137, special=137435137)
|     at keyboard.c:10014
| #23 0x080f6142 in command_loop_1 () at keyboard.c:1873
| #24 0x081470ae in internal_condition_case (bfun=0x80f5dd0 <command_loop_1>, handlers=137482865,
|     hfun=0x80ef968 <cmd_error>) at eval.c:1481
| #25 0x080e9f26 in command_loop_2 () at keyboard.c:1329
| #26 0x08146dd5 in internal_catch (tag=127, func=0x80e9f08 <command_loop_2>, arg=137435137) at eval.c:1222
| #27 0x080e9d65 in command_loop () at keyboard.c:1308
| #28 0x080e9e00 in recursive_edit_1 () at keyboard.c:1006
| #29 0x080e9eca in Frecursive_edit () at keyboard.c:1067
| #30 0x080e9352 in main (argc=3, argv=0xbfbfe828) at emacs.c:1761
|
| Lisp Backtrace:
| "iconify-frame" (0x8311831)
| "iconify-or-deiconify-frame" (0x8311801)
| "call-interactively" (0x84fe861)
| (gdb)
|
`-----------------------------------------------------------------------

This GTK+-enabled Emacs has been compiled in ~/tmp/emacs-22.0.95
with the following configure-time options:

  ./configure --prefix=/opt/emacs --with-x --with-x-toolkit=gtk \
    --with-xpm --with-jpeg --with-tiff --with-gif --with-png

I can upload the full config.log and 'emacs-22.0.95.log' file if
it helps, but I don't really know how to track this down to its
real cause.

FWIW, the crash is not repeatable when Emacs is built with the
Lucid widget set.

- Giorgos

Re: Pretest?

[Chong Yidong]

Giorgos Keramidas <keramida@ceid.upatras.gr> writes:

>   FreeBSD 7.0-CURRENT #0: Tue Feb 27 01:25:46 EET 2007
>
> While I'm running the GTK+ version, however, I can crash Emacs in
> emacs_blocked_free() by following the steps outlined below:
>
> * Run Emacs inside gdb:
> * Run M-x gnus-agent-batch while my network connection is
>   disabled, and let it time-out.  It prompts me for going into
>   `off-line mode', to which I reply `yes'.
> * The next time I input C-z Emacs crashes with a backtrace of:

I can't seem to reproduce this on GNU/Linux (I don't have a FreeBSD
box handy).  It's strange that gnus-agent batch has anything to do
with it.  Have you been able to reproduce the C-z crash in any other
circumstance?  (It is better to get a recipe not involving gnus, since
that might depend on your newsgroup settings.)

In any case, the backtrace indicates that the crash occurs deep in
GTK/Glib.  If you look at what is occurring in the Emacs code, what
we're doing is perfectly innocuous: gdk_pixbuf_new_from_xpm_data() is
called on a static character array containing an XPM image.  So the
bug is probably in GTK or Glib, not in Emacs.

Re: Pretest?

[Giorgos Keramidas]

On 2007-03-04 15:25, Chong Yidong <cyd@stupidchicken.com> wrote:
>Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
>>   FreeBSD 7.0-CURRENT #0: Tue Feb 27 01:25:46 EET 2007
>>
>> While I'm running the GTK+ version, however, I can crash Emacs in
>> emacs_blocked_free() by following the steps outlined below:
>>
>> * Run Emacs inside gdb:
>> * Run M-x gnus-agent-batch while my network connection is
>>   disabled, and let it time-out.  It prompts me for going into
>>   `off-line mode', to which I reply `yes'.
>> * The next time I input C-z Emacs crashes with a backtrace of:
> 
> I can't seem to reproduce this on GNU/Linux (I don't have a FreeBSD
> box handy).  It's strange that gnus-agent batch has anything to do
> with it.  Have you been able to reproduce the C-z crash in any other
> circumstance?  (It is better to get a recipe not involving gnus, since
> that might depend on your newsgroup settings.)

I haven't been able to reproduce this consistently, and most of the
times I fire up Emacs I also start Gnus.  I'll try to see if I can
reproduce this without running Gnus at all.

> In any case, the backtrace indicates that the crash occurs deep in
> GTK/Glib.  If you look at what is occurring in the Emacs code, what
> we're doing is perfectly innocuous: gdk_pixbuf_new_from_xpm_data() is
> called on a static character array containing an XPM image.  So the
> bug is probably in GTK or Glib, not in Emacs.

Ok, I'm not sure if this is an Emacs bug.  It is definitely *not*
reproducible with any other toolkit, so it may be ok for the update of
the package/port to 22.0.95 to use a warning in the package installation
which mentions that 'make WITH_GTK=yes' *may* cause problems.

Thanks for the help :)

- Giorgos

Re: Pretest?

[David Kastrup]

Chong Yidong <cyd@stupidchicken.com> writes:

> Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
>
>>   FreeBSD 7.0-CURRENT #0: Tue Feb 27 01:25:46 EET 2007
>>
>> While I'm running the GTK+ version, however, I can crash Emacs in
>> emacs_blocked_free() by following the steps outlined below:
>>
>> * Run Emacs inside gdb:
>> * Run M-x gnus-agent-batch while my network connection is
>>   disabled, and let it time-out.  It prompts me for going into
>>   `off-line mode', to which I reply `yes'.
>> * The next time I input C-z Emacs crashes with a backtrace of:
>
> I can't seem to reproduce this on GNU/Linux (I don't have a FreeBSD
> box handy).  It's strange that gnus-agent batch has anything to do
> with it.  Have you been able to reproduce the C-z crash in any other
> circumstance?  (It is better to get a recipe not involving gnus, since
> that might depend on your newsgroup settings.)
>
> In any case, the backtrace indicates that the crash occurs deep in
> GTK/Glib.  If you look at what is occurring in the Emacs code, what
> we're doing is perfectly innocuous: gdk_pixbuf_new_from_xpm_data() is
> called on a static character array containing an XPM image.  So the
> bug is probably in GTK or Glib, not in Emacs.

Could not possibly have anything to do with the following?

2007-03-01  Kenichi Handa  <handa@m17n.org>

	* process.c (send_process_object): Check the process status and
	signal an error if something is wrong.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Pretest?

[Giorgos Keramidas]

On 2007-03-04 21:38, David Kastrup <dak@gnu.org> wrote:
>Chong Yidong <cyd@stupidchicken.com> writes:
>> Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
>>
>>>   FreeBSD 7.0-CURRENT #0: Tue Feb 27 01:25:46 EET 2007
>>>
>>> While I'm running the GTK+ version, however, I can crash Emacs in
>>> emacs_blocked_free() by following the steps outlined below:
>>>
>>> * Run Emacs inside gdb:
>>> * Run M-x gnus-agent-batch while my network connection is
>>>   disabled, and let it time-out.  It prompts me for going into
>>>   `off-line mode', to which I reply `yes'.
>>> * The next time I input C-z Emacs crashes with a backtrace of:
>>
>> I can't seem to reproduce this on GNU/Linux (I don't have a FreeBSD
>> box handy).  It's strange that gnus-agent batch has anything to do
>> with it.  Have you been able to reproduce the C-z crash in any other
>> circumstance?  (It is better to get a recipe not involving gnus, since
>> that might depend on your newsgroup settings.)
>>
>> In any case, the backtrace indicates that the crash occurs deep in
>> GTK/Glib.  If you look at what is occurring in the Emacs code, what
>> we're doing is perfectly innocuous: gdk_pixbuf_new_from_xpm_data() is
>> called on a static character array containing an XPM image.  So the
>> bug is probably in GTK or Glib, not in Emacs.
>
> Could not possibly have anything to do with the following?
>
> 2007-03-01  Kenichi Handa  <handa@m17n.org>
>
> 	* process.c (send_process_object): Check the process status and
> 	signal an error if something is wrong.

I sort of doubt that, but I can test.

IIRC, Emacs 22.X had problems on FreeBSD with GTK+ a long time ago too.

Re: Pretest?

[Jan Djärv]

Giorgos Keramidas skrev:
> On 2007-03-04 21:38, David Kastrup <dak@gnu.org> wrote:
>> Chong Yidong <cyd@stupidchicken.com> writes:
>>> Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
>>>
>>>>   FreeBSD 7.0-CURRENT #0: Tue Feb 27 01:25:46 EET 2007
>>>>
>>>> While I'm running the GTK+ version, however, I can crash Emacs in
>>>> emacs_blocked_free() by following the steps outlined below:
>>>>
>>>> * Run Emacs inside gdb:
>>>> * Run M-x gnus-agent-batch while my network connection is
>>>>   disabled, and let it time-out.  It prompts me for going into
>>>>   `off-line mode', to which I reply `yes'.
>>>> * The next time I input C-z Emacs crashes with a backtrace of:
>>> I can't seem to reproduce this on GNU/Linux (I don't have a FreeBSD
>>> box handy).  It's strange that gnus-agent batch has anything to do
>>> with it.  Have you been able to reproduce the C-z crash in any other
>>> circumstance?  (It is better to get a recipe not involving gnus, since
>>> that might depend on your newsgroup settings.)
>>>
>>> In any case, the backtrace indicates that the crash occurs deep in
>>> GTK/Glib.  If you look at what is occurring in the Emacs code, what
>>> we're doing is perfectly innocuous: gdk_pixbuf_new_from_xpm_data() is
>>> called on a static character array containing an XPM image.  So the
>>> bug is probably in GTK or Glib, not in Emacs.
>> Could not possibly have anything to do with the following?
>>
>> 2007-03-01  Kenichi Handa  <handa@m17n.org>
>>
>> 	* process.c (send_process_object): Check the process status and
>> 	signal an error if something is wrong.
> 
> I sort of doubt that, but I can test.
> 
> IIRC, Emacs 22.X had problems on FreeBSD with GTK+ a long time ago too.

As I recall, it was the threading issue that was the problem then.  Are there 
multiple threads running in Emacs at the time of the crash?  info threads in 
gdb will show you that.  A backtrace for each thread, if there are several, 
would be nice.

	Jan D.

Re: Pretest?

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 05 Mar 2007 08:15:40 +0100, Jan Djärv <jan.h.d@swipnet.se> said:

>> IIRC, Emacs 22.X had problems on FreeBSD with GTK+ a long time ago
>> too.

> As I recall, it was the threading issue that was the problem then.
> Are there multiple threads running in Emacs at the time of the
> crash?  info threads in gdb will show you that.  A backtrace for
> each thread, if there are several, would be nice.

I suspect src/gmalloc.c, which does not provide thread-safe malloc
(correct me if I'm wrong), is used while HAVE_GTK_AND_PTHREAD is
defined.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Pretest?

[Jan Djärv]

YAMAMOTO Mitsuharu skrev:
>>>>>> On Mon, 05 Mar 2007 08:15:40 +0100, Jan Djärv <jan.h.d@swipnet.se> said:
> 
>>> IIRC, Emacs 22.X had problems on FreeBSD with GTK+ a long time ago
>>> too.
> 
>> As I recall, it was the threading issue that was the problem then.
>> Are there multiple threads running in Emacs at the time of the
>> crash?  info threads in gdb will show you that.  A backtrace for
>> each thread, if there are several, would be nice.
> 
> I suspect src/gmalloc.c, which does not provide thread-safe malloc
> (correct me if I'm wrong), is used while HAVE_GTK_AND_PTHREAD is
> defined.

If src/gmalloc.c is used, then the hooks in alloc.c are used, so they should 
be thread safe.

	Jan D.

Re: Pretest?

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 15 Mar 2007 12:04:33 +0100, Jan Djärv <jan.h.d@swipnet.se> said:

>> I suspect src/gmalloc.c, which does not provide thread-safe malloc
>> (correct me if I'm wrong), is used while HAVE_GTK_AND_PTHREAD is
>> defined.

> If src/gmalloc.c is used, then the hooks in alloc.c are used, so
> they should be thread safe.

No, alloc_mutex only protects the variables __malloc_hook etc., but
not for the heap itself.  Consider the following scenario:

  1) The main thread calls malloc.
  2) It then calls emacs_blocked_malloc via __malloc_hook.
  3) __malloc_hook is set to old_malloc_hook in emacs_blocked_malloc.
  4) Now the main thread is in execution of the original malloc.
  5) Another thread also calls malloc.
  6) The thread also executes the original malloc because
     __malloc_hook == old_malloc_hook.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Pretest?

[Jan Djärv]

YAMAMOTO Mitsuharu skrev:
>>>>>> On Thu, 15 Mar 2007 12:04:33 +0100, Jan Djärv <jan.h.d@swipnet.se> said:
> 
>>> I suspect src/gmalloc.c, which does not provide thread-safe malloc
>>> (correct me if I'm wrong), is used while HAVE_GTK_AND_PTHREAD is
>>> defined.
> 
>> If src/gmalloc.c is used, then the hooks in alloc.c are used, so
>> they should be thread safe.
> 
> No, alloc_mutex only protects the variables __malloc_hook etc., but
> not for the heap itself.  Consider the following scenario:
> 
>   1) The main thread calls malloc.
>   2) It then calls emacs_blocked_malloc via __malloc_hook.
>   3) __malloc_hook is set to old_malloc_hook in emacs_blocked_malloc.
>   4) Now the main thread is in execution of the original malloc.
>   5) Another thread also calls malloc.
>   6) The thread also executes the original malloc because
>      __malloc_hook == old_malloc_hook.

You are correct.  Can we, should we fix this, i.e. make gmalloc.c thread safe?

	Jan D.

Re: Pretest?

[Jan Djärv]

Jan Djärv skrev:
> 
> 
> YAMAMOTO Mitsuharu skrev:
>>>>>>> On Thu, 15 Mar 2007 12:04:33 +0100, Jan Djärv 
>>>>>>> <jan.h.d@swipnet.se> said:
>>
>>>> I suspect src/gmalloc.c, which does not provide thread-safe malloc
>>>> (correct me if I'm wrong), is used while HAVE_GTK_AND_PTHREAD is
>>>> defined.
>>
>>> If src/gmalloc.c is used, then the hooks in alloc.c are used, so
>>> they should be thread safe.
>>
>> No, alloc_mutex only protects the variables __malloc_hook etc., but
>> not for the heap itself.  Consider the following scenario:
>>
>>   1) The main thread calls malloc.
>>   2) It then calls emacs_blocked_malloc via __malloc_hook.
>>   3) __malloc_hook is set to old_malloc_hook in emacs_blocked_malloc.
>>   4) Now the main thread is in execution of the original malloc.
>>   5) Another thread also calls malloc.
>>   6) The thread also executes the original malloc because
>>      __malloc_hook == old_malloc_hook.
> 
> You are correct.  Can we, should we fix this, i.e. make gmalloc.c thread 
> safe?
> 

A thing to do after the release might be to synchronize with the thread safe 
malloc in glibc.

	Jan D.

Re: Pretest?

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 19 Mar 2007 09:00:10 +0100, Jan Djärv <jan.h.d@swipnet.se> said:

> A thing to do after the release might be to synchronize with the
> thread safe malloc in glibc.

What do we do before the release?  Possible options would be:

  1) Use the system malloc library (and abandon emacs_blocked_*) if
     HAVE_GTK_AND_PTHREAD.
  2) Modify src/gmalloc.c to make it thread safe.

I just tried the latter, but I can't test it myself.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Index: src/gmalloc.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/gmalloc.c,v
retrieving revision 1.20
diff -c -p -r1.20 gmalloc.c
*** src/gmalloc.c	21 Jan 2007 04:18:16 -0000	1.20
--- src/gmalloc.c	19 Mar 2007 09:18:53 -0000
***************
*** 1,6 ****
--- 1,9 ----
  /* This file is no longer automatically generated from libc.  */
  
  #define _MALLOC_INTERNAL
+ #ifdef HAVE_GTK_AND_PTHREAD
+ #define USE_PTHREAD
+ #endif
  
  /* The malloc headers and source files from the C library follow here.  */
  
*************** Fifth Floor, Boston, MA 02110-1301, USA.
*** 73,78 ****
--- 76,85 ----
  #include <unistd.h>
  #endif
  
+ #ifdef USE_PTHREAD
+ #include <pthread.h>
+ #endif
+ 
  #endif	/* _MALLOC_INTERNAL.  */
  
  
*************** extern __ptr_t _malloc_internal PP ((__m
*** 229,234 ****
--- 236,244 ----
  extern __ptr_t _realloc_internal PP ((__ptr_t __ptr, __malloc_size_t __size));
  extern void _free_internal PP ((__ptr_t __ptr));
  
+ #ifdef USE_PTHREAD
+ extern pthread_mutex_t _malloc_mutex;
+ #endif
  #endif /* _MALLOC_INTERNAL.  */
  
  /* Given an address in the middle of a malloc'd object,
*************** register_heapinfo ()
*** 536,548 ****
      _heapinfo[block + blocks].busy.info.size = -blocks;
  }
  
! /* Set everything up and remember that we have.  */
! int
! __malloc_initialize ()
! {
!   if (__malloc_initialized)
!     return 0;
  
  #ifdef GC_MCHECK
    mcheck (NULL);
  #endif
--- 546,559 ----
      _heapinfo[block + blocks].busy.info.size = -blocks;
  }
  
! #ifdef USE_PTHREAD
! static pthread_once_t malloc_init_once_control = PTHREAD_ONCE_INIT;
! pthread_mutex_t _malloc_mutex;
! #endif
  
+ static void
+ malloc_initialize_1 ()
+ {
  #ifdef GC_MCHECK
    mcheck (NULL);
  #endif
*************** __malloc_initialize ()
*** 550,559 ****
    if (__malloc_initialize_hook)
      (*__malloc_initialize_hook) ();
  
    heapsize = HEAP / BLOCKSIZE;
    _heapinfo = (malloc_info *) align (heapsize * sizeof (malloc_info));
    if (_heapinfo == NULL)
!     return 0;
    memset (_heapinfo, 0, heapsize * sizeof (malloc_info));
    _heapinfo[0].free.size = 0;
    _heapinfo[0].free.next = _heapinfo[0].free.prev = 0;
--- 561,581 ----
    if (__malloc_initialize_hook)
      (*__malloc_initialize_hook) ();
  
+ #ifdef USE_PTHREAD
+   {
+     pthread_mutexattr_t attr;
+ 
+     pthread_mutexattr_init (&attr);
+     pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
+     pthread_mutex_init (&_malloc_mutex, &attr);
+     pthread_mutexattr_destroy (&attr);
+   }
+ #endif
+ 
    heapsize = HEAP / BLOCKSIZE;
    _heapinfo = (malloc_info *) align (heapsize * sizeof (malloc_info));
    if (_heapinfo == NULL)
!     return;
    memset (_heapinfo, 0, heapsize * sizeof (malloc_info));
    _heapinfo[0].free.size = 0;
    _heapinfo[0].free.next = _heapinfo[0].free.prev = 0;
*************** __malloc_initialize ()
*** 565,571 ****
  
    __malloc_initialized = 1;
    PROTECT_MALLOC_STATE (1);
!   return 1;
  }
  
  static int morecore_recursing;
--- 587,609 ----
  
    __malloc_initialized = 1;
    PROTECT_MALLOC_STATE (1);
!   return;
! }
! 
! /* Set everything up and remember that we have.  */
! int
! __malloc_initialize ()
! {
! #ifdef USE_PTHREAD
!   pthread_once (&malloc_init_once_control, malloc_initialize_1);
! #else
!   if (__malloc_initialized)
!     return 0;
! 
!   malloc_initialize_1 ();
! #endif
! 
!   return __malloc_initialized;
  }
  
  static int morecore_recursing;
*************** _malloc_internal (size)
*** 708,713 ****
--- 746,754 ----
      return NULL;
  #endif
  
+ #ifdef USE_PTHREAD
+   pthread_mutex_lock (&_malloc_mutex);
+ #endif
    PROTECT_MALLOC_STATE (0);
  
    if (size < sizeof (struct list))
*************** _malloc_internal (size)
*** 765,771 ****
  	  if (result == NULL)
  	    {
  	      PROTECT_MALLOC_STATE (1);
! 	      return NULL;
  	    }
  
  	  /* Link all fragments but the first into the free list.  */
--- 806,812 ----
  	  if (result == NULL)
  	    {
  	      PROTECT_MALLOC_STATE (1);
! 	      goto out;
  	    }
  
  	  /* Link all fragments but the first into the free list.  */
*************** _malloc_internal (size)
*** 831,837 ****
  		}
  	      result = morecore (wantblocks * BLOCKSIZE);
  	      if (result == NULL)
! 		return NULL;
  	      block = BLOCK (result);
  	      /* Put the new block at the end of the free list.  */
  	      _heapinfo[block].free.size = wantblocks;
--- 872,878 ----
  		}
  	      result = morecore (wantblocks * BLOCKSIZE);
  	      if (result == NULL)
! 		goto out;
  	      block = BLOCK (result);
  	      /* Put the new block at the end of the free list.  */
  	      _heapinfo[block].free.size = wantblocks;
*************** _malloc_internal (size)
*** 886,891 ****
--- 927,936 ----
      }
  
    PROTECT_MALLOC_STATE (1);
+  out:
+ #ifdef USE_PTHREAD
+   pthread_mutex_unlock (&_malloc_mutex);
+ #endif
    return result;
  }
  
*************** _free_internal (ptr)
*** 996,1001 ****
--- 1041,1049 ----
    if (ptr == NULL)
      return;
  
+ #ifdef USE_PTHREAD
+   pthread_mutex_lock (&_malloc_mutex);
+ #endif
    PROTECT_MALLOC_STATE (0);
  
    for (l = _aligned_blocks; l != NULL; l = l->next)
*************** _free_internal (ptr)
*** 1221,1226 ****
--- 1269,1277 ----
      }
  
    PROTECT_MALLOC_STATE (1);
+ #ifdef USE_PTHREAD
+   pthread_mutex_unlock (&_malloc_mutex);
+ #endif
  }
  
  /* Return memory to the heap.  */
*************** _realloc_internal (ptr, size)
*** 1384,1389 ****
--- 1435,1443 ----
  
    block = BLOCK (ptr);
  
+ #ifdef USE_PTHREAD
+   pthread_mutex_lock (&_malloc_mutex);
+ #endif
    PROTECT_MALLOC_STATE (0);
  
    type = _heapinfo[block].busy.type;
*************** _realloc_internal (ptr, size)
*** 1398,1404 ****
  	    {
  	      memcpy (result, ptr, size);
  	      _free_internal (ptr);
! 	      return result;
  	    }
  	}
  
--- 1452,1458 ----
  	    {
  	      memcpy (result, ptr, size);
  	      _free_internal (ptr);
! 	      goto out;
  	    }
  	}
  
*************** _realloc_internal (ptr, size)
*** 1451,1457 ****
  		  (void) _malloc_internal (blocks * BLOCKSIZE);
  		  _free_internal (previous);
  		}
! 	      return NULL;
  	    }
  	  if (ptr != result)
  	    memmove (result, ptr, blocks * BLOCKSIZE);
--- 1505,1511 ----
  		  (void) _malloc_internal (blocks * BLOCKSIZE);
  		  _free_internal (previous);
  		}
! 	      goto out;
  	    }
  	  if (ptr != result)
  	    memmove (result, ptr, blocks * BLOCKSIZE);
*************** _realloc_internal (ptr, size)
*** 1471,1477 ****
  	     and copy the lesser of the new size and the old. */
  	  result = _malloc_internal (size);
  	  if (result == NULL)
! 	    return NULL;
  	  memcpy (result, ptr, min (size, (__malloc_size_t) 1 << type));
  	  _free_internal (ptr);
  	}
--- 1525,1531 ----
  	     and copy the lesser of the new size and the old. */
  	  result = _malloc_internal (size);
  	  if (result == NULL)
! 	    goto out;
  	  memcpy (result, ptr, min (size, (__malloc_size_t) 1 << type));
  	  _free_internal (ptr);
  	}
*************** _realloc_internal (ptr, size)
*** 1479,1484 ****
--- 1533,1542 ----
      }
  
    PROTECT_MALLOC_STATE (1);
+  out:
+ #ifdef USE_PTHREAD
+   pthread_mutex_unlock (&_malloc_mutex);
+ #endif
    return result;
  }
  

Re: Pretest?

[Giorgos Keramidas]

On 2007-03-19 18:31, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> wrote:
> >>>>> On Mon, 19 Mar 2007 09:00:10 +0100, Jan Dj?rv <jan.h.d@swipnet.se> said:
> 
> > A thing to do after the release might be to synchronize with the
> > thread safe malloc in glibc.
> 
> What do we do before the release?  Possible options would be:
> 
>   1) Use the system malloc library (and abandon emacs_blocked_*) if
>      HAVE_GTK_AND_PTHREAD.
>   2) Modify src/gmalloc.c to make it thread safe.
> 
> I just tried the latter, but I can't test it myself.

Thanks for the patch :-)

Since I was the one reporting instability with Emacs compiled
with GTK+ support, I can roll an Emacs build with GTK+ enabled,
and see if this makes things more stable here.

I just updated my local copy of the CVS repository with all
changes up to:

    changeset:   80041:df347cacb5a9
    tag:         tip
    user:        gm
    date:        Mon Mar 19 20:59:53 2007 +0000
    summary:     Change form of license text to match rest of Emacs.

and I'm building a new snapshot now...

> Index: src/gmalloc.c
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/src/gmalloc.c,v
> retrieving revision 1.20
> diff -c -p -r1.20 gmalloc.c
> *** src/gmalloc.c	21 Jan 2007 04:18:16 -0000	1.20
> --- src/gmalloc.c	19 Mar 2007 09:18:53 -0000
> ***************
> *** 1,6 ****
> --- 1,9 ----
>   /* This file is no longer automatically generated from libc.  */
>   
>   #define _MALLOC_INTERNAL
> + #ifdef HAVE_GTK_AND_PTHREAD
> + #define USE_PTHREAD
> + #endif
>   
>   /* The malloc headers and source files from the C library follow here.  */
>   
> *************** Fifth Floor, Boston, MA 02110-1301, USA.
> *** 73,78 ****
> --- 76,85 ----
>   #include <unistd.h>
>   #endif
>   
> + #ifdef USE_PTHREAD
> + #include <pthread.h>
> + #endif
> + 
>   #endif	/* _MALLOC_INTERNAL.  */
>   
>   
> *************** extern __ptr_t _malloc_internal PP ((__m
> *** 229,234 ****
> --- 236,244 ----
>   extern __ptr_t _realloc_internal PP ((__ptr_t __ptr, __malloc_size_t __size));
>   extern void _free_internal PP ((__ptr_t __ptr));
>   
> + #ifdef USE_PTHREAD
> + extern pthread_mutex_t _malloc_mutex;
> + #endif
>   #endif /* _MALLOC_INTERNAL.  */
>   
>   /* Given an address in the middle of a malloc'd object,
> *************** register_heapinfo ()
> *** 536,548 ****
>       _heapinfo[block + blocks].busy.info.size = -blocks;
>   }
>   
> ! /* Set everything up and remember that we have.  */
> ! int
> ! __malloc_initialize ()
> ! {
> !   if (__malloc_initialized)
> !     return 0;
>   
>   #ifdef GC_MCHECK
>     mcheck (NULL);
>   #endif
> --- 546,559 ----
>       _heapinfo[block + blocks].busy.info.size = -blocks;
>   }
>   
> ! #ifdef USE_PTHREAD
> ! static pthread_once_t malloc_init_once_control = PTHREAD_ONCE_INIT;
> ! pthread_mutex_t _malloc_mutex;
> ! #endif
>   
> + static void
> + malloc_initialize_1 ()
> + {
>   #ifdef GC_MCHECK
>     mcheck (NULL);
>   #endif
> *************** __malloc_initialize ()
> *** 550,559 ****
>     if (__malloc_initialize_hook)
>       (*__malloc_initialize_hook) ();
>   
>     heapsize = HEAP / BLOCKSIZE;
>     _heapinfo = (malloc_info *) align (heapsize * sizeof (malloc_info));
>     if (_heapinfo == NULL)
> !     return 0;
>     memset (_heapinfo, 0, heapsize * sizeof (malloc_info));
>     _heapinfo[0].free.size = 0;
>     _heapinfo[0].free.next = _heapinfo[0].free.prev = 0;
> --- 561,581 ----
>     if (__malloc_initialize_hook)
>       (*__malloc_initialize_hook) ();
>   
> + #ifdef USE_PTHREAD
> +   {
> +     pthread_mutexattr_t attr;
> + 
> +     pthread_mutexattr_init (&attr);
> +     pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
> +     pthread_mutex_init (&_malloc_mutex, &attr);
> +     pthread_mutexattr_destroy (&attr);
> +   }
> + #endif
> + 
>     heapsize = HEAP / BLOCKSIZE;
>     _heapinfo = (malloc_info *) align (heapsize * sizeof (malloc_info));
>     if (_heapinfo == NULL)
> !     return;
>     memset (_heapinfo, 0, heapsize * sizeof (malloc_info));
>     _heapinfo[0].free.size = 0;
>     _heapinfo[0].free.next = _heapinfo[0].free.prev = 0;
> *************** __malloc_initialize ()
> *** 565,571 ****
>   
>     __malloc_initialized = 1;
>     PROTECT_MALLOC_STATE (1);
> !   return 1;
>   }
>   
>   static int morecore_recursing;
> --- 587,609 ----
>   
>     __malloc_initialized = 1;
>     PROTECT_MALLOC_STATE (1);
> !   return;
> ! }
> ! 
> ! /* Set everything up and remember that we have.  */
> ! int
> ! __malloc_initialize ()
> ! {
> ! #ifdef USE_PTHREAD
> !   pthread_once (&malloc_init_once_control, malloc_initialize_1);
> ! #else
> !   if (__malloc_initialized)
> !     return 0;
> ! 
> !   malloc_initialize_1 ();
> ! #endif
> ! 
> !   return __malloc_initialized;
>   }
>   
>   static int morecore_recursing;
> *************** _malloc_internal (size)
> *** 708,713 ****
> --- 746,754 ----
>       return NULL;
>   #endif
>   
> + #ifdef USE_PTHREAD
> +   pthread_mutex_lock (&_malloc_mutex);
> + #endif
>     PROTECT_MALLOC_STATE (0);
>   
>     if (size < sizeof (struct list))
> *************** _malloc_internal (size)
> *** 765,771 ****
>   	  if (result == NULL)
>   	    {
>   	      PROTECT_MALLOC_STATE (1);
> ! 	      return NULL;
>   	    }
>   
>   	  /* Link all fragments but the first into the free list.  */
> --- 806,812 ----
>   	  if (result == NULL)
>   	    {
>   	      PROTECT_MALLOC_STATE (1);
> ! 	      goto out;
>   	    }
>   
>   	  /* Link all fragments but the first into the free list.  */
> *************** _malloc_internal (size)
> *** 831,837 ****
>   		}
>   	      result = morecore (wantblocks * BLOCKSIZE);
>   	      if (result == NULL)
> ! 		return NULL;
>   	      block = BLOCK (result);
>   	      /* Put the new block at the end of the free list.  */
>   	      _heapinfo[block].free.size = wantblocks;
> --- 872,878 ----
>   		}
>   	      result = morecore (wantblocks * BLOCKSIZE);
>   	      if (result == NULL)
> ! 		goto out;
>   	      block = BLOCK (result);
>   	      /* Put the new block at the end of the free list.  */
>   	      _heapinfo[block].free.size = wantblocks;
> *************** _malloc_internal (size)
> *** 886,891 ****
> --- 927,936 ----
>       }
>   
>     PROTECT_MALLOC_STATE (1);
> +  out:
> + #ifdef USE_PTHREAD
> +   pthread_mutex_unlock (&_malloc_mutex);
> + #endif
>     return result;
>   }
>   
> *************** _free_internal (ptr)
> *** 996,1001 ****
> --- 1041,1049 ----
>     if (ptr == NULL)
>       return;
>   
> + #ifdef USE_PTHREAD
> +   pthread_mutex_lock (&_malloc_mutex);
> + #endif
>     PROTECT_MALLOC_STATE (0);
>   
>     for (l = _aligned_blocks; l != NULL; l = l->next)
> *************** _free_internal (ptr)
> *** 1221,1226 ****
> --- 1269,1277 ----
>       }
>   
>     PROTECT_MALLOC_STATE (1);
> + #ifdef USE_PTHREAD
> +   pthread_mutex_unlock (&_malloc_mutex);
> + #endif
>   }
>   
>   /* Return memory to the heap.  */
> *************** _realloc_internal (ptr, size)
> *** 1384,1389 ****
> --- 1435,1443 ----
>   
>     block = BLOCK (ptr);
>   
> + #ifdef USE_PTHREAD
> +   pthread_mutex_lock (&_malloc_mutex);
> + #endif
>     PROTECT_MALLOC_STATE (0);
>   
>     type = _heapinfo[block].busy.type;
> *************** _realloc_internal (ptr, size)
> *** 1398,1404 ****
>   	    {
>   	      memcpy (result, ptr, size);
>   	      _free_internal (ptr);
> ! 	      return result;
>   	    }
>   	}
>   
> --- 1452,1458 ----
>   	    {
>   	      memcpy (result, ptr, size);
>   	      _free_internal (ptr);
> ! 	      goto out;
>   	    }
>   	}
>   
> *************** _realloc_internal (ptr, size)
> *** 1451,1457 ****
>   		  (void) _malloc_internal (blocks * BLOCKSIZE);
>   		  _free_internal (previous);
>   		}
> ! 	      return NULL;
>   	    }
>   	  if (ptr != result)
>   	    memmove (result, ptr, blocks * BLOCKSIZE);
> --- 1505,1511 ----
>   		  (void) _malloc_internal (blocks * BLOCKSIZE);
>   		  _free_internal (previous);
>   		}
> ! 	      goto out;
>   	    }
>   	  if (ptr != result)
>   	    memmove (result, ptr, blocks * BLOCKSIZE);
> *************** _realloc_internal (ptr, size)
> *** 1471,1477 ****
>   	     and copy the lesser of the new size and the old. */
>   	  result = _malloc_internal (size);
>   	  if (result == NULL)
> ! 	    return NULL;
>   	  memcpy (result, ptr, min (size, (__malloc_size_t) 1 << type));
>   	  _free_internal (ptr);
>   	}
> --- 1525,1531 ----
>   	     and copy the lesser of the new size and the old. */
>   	  result = _malloc_internal (size);
>   	  if (result == NULL)
> ! 	    goto out;
>   	  memcpy (result, ptr, min (size, (__malloc_size_t) 1 << type));
>   	  _free_internal (ptr);
>   	}
> *************** _realloc_internal (ptr, size)
> *** 1479,1484 ****
> --- 1533,1542 ----
>       }
>   
>     PROTECT_MALLOC_STATE (1);
> +  out:
> + #ifdef USE_PTHREAD
> +   pthread_mutex_unlock (&_malloc_mutex);
> + #endif
>     return result;
>   }
>   
> 
> 
> _______________________________________________
> Emacs-devel mailing list
> Emacs-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: Pretest?

[Jan Djärv]

YAMAMOTO Mitsuharu skrev:
>>>>>> On Mon, 19 Mar 2007 09:00:10 +0100, Jan Djärv <jan.h.d@swipnet.se> said:
> 
>> A thing to do after the release might be to synchronize with the
>> thread safe malloc in glibc.
> 
> What do we do before the release?  Possible options would be:
> 
>   1) Use the system malloc library (and abandon emacs_blocked_*) if
>      HAVE_GTK_AND_PTHREAD.
>   2) Modify src/gmalloc.c to make it thread safe.
> 
> I just tried the latter, but I can't test it myself.

I'll test it, but I can just test that it works, I haven't seen any problem 
myself.

However, as a matter of style, I prefer to use
#ifdef USE_PTHREAD
#define LOCK()     pthread_mutex_lock (&_malloc_mutex)
#define UNLOCK()   pthread_mutex_unlock (&_malloc_mutex)
#else
#define LOCK()
#define UNLOCK()
#endif

and then use LOCK/UNLOCK without any #ifdef:s in the functions.  It is so much 
easier later if you need to read the code or insert some debugging info in to 
the lock/unlock phase.

	Jan D.

> 
> 				     YAMAMOTO Mitsuharu
> 				mituharu@math.s.chiba-u.ac.jp
> 
> Index: src/gmalloc.c
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/src/gmalloc.c,v
> retrieving revision 1.20
> diff -c -p -r1.20 gmalloc.c
> *** src/gmalloc.c	21 Jan 2007 04:18:16 -0000	1.20
> --- src/gmalloc.c	19 Mar 2007 09:18:53 -0000
> ***************
> *** 1,6 ****
> --- 1,9 ----
>   /* This file is no longer automatically generated from libc.  */
>   
>   #define _MALLOC_INTERNAL
> + #ifdef HAVE_GTK_AND_PTHREAD
> + #define USE_PTHREAD
> + #endif
>   
>   /* The malloc headers and source files from the C library follow here.  */
>   
> *************** Fifth Floor, Boston, MA 02110-1301, USA.
> *** 73,78 ****
> --- 76,85 ----
>   #include <unistd.h>
>   #endif
>   
> + #ifdef USE_PTHREAD
> + #include <pthread.h>
> + #endif
> + 
>   #endif	/* _MALLOC_INTERNAL.  */
>   
>   
> *************** extern __ptr_t _malloc_internal PP ((__m
> *** 229,234 ****
> --- 236,244 ----
>   extern __ptr_t _realloc_internal PP ((__ptr_t __ptr, __malloc_size_t __size));
>   extern void _free_internal PP ((__ptr_t __ptr));
>   
> + #ifdef USE_PTHREAD
> + extern pthread_mutex_t _malloc_mutex;
> + #endif
>   #endif /* _MALLOC_INTERNAL.  */
>   
>   /* Given an address in the middle of a malloc'd object,
> *************** register_heapinfo ()
> *** 536,548 ****
>       _heapinfo[block + blocks].busy.info.size = -blocks;
>   }
>   
> ! /* Set everything up and remember that we have.  */
> ! int
> ! __malloc_initialize ()
> ! {
> !   if (__malloc_initialized)
> !     return 0;
>   
>   #ifdef GC_MCHECK
>     mcheck (NULL);
>   #endif
> --- 546,559 ----
>       _heapinfo[block + blocks].busy.info.size = -blocks;
>   }
>   
> ! #ifdef USE_PTHREAD
> ! static pthread_once_t malloc_init_once_control = PTHREAD_ONCE_INIT;
> ! pthread_mutex_t _malloc_mutex;
> ! #endif
>   
> + static void
> + malloc_initialize_1 ()
> + {
>   #ifdef GC_MCHECK
>     mcheck (NULL);
>   #endif
> *************** __malloc_initialize ()
> *** 550,559 ****
>     if (__malloc_initialize_hook)
>       (*__malloc_initialize_hook) ();
>   
>     heapsize = HEAP / BLOCKSIZE;
>     _heapinfo = (malloc_info *) align (heapsize * sizeof (malloc_info));
>     if (_heapinfo == NULL)
> !     return 0;
>     memset (_heapinfo, 0, heapsize * sizeof (malloc_info));
>     _heapinfo[0].free.size = 0;
>     _heapinfo[0].free.next = _heapinfo[0].free.prev = 0;
> --- 561,581 ----
>     if (__malloc_initialize_hook)
>       (*__malloc_initialize_hook) ();
>   
> + #ifdef USE_PTHREAD
> +   {
> +     pthread_mutexattr_t attr;
> + 
> +     pthread_mutexattr_init (&attr);
> +     pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
> +     pthread_mutex_init (&_malloc_mutex, &attr);
> +     pthread_mutexattr_destroy (&attr);
> +   }
> + #endif
> + 
>     heapsize = HEAP / BLOCKSIZE;
>     _heapinfo = (malloc_info *) align (heapsize * sizeof (malloc_info));
>     if (_heapinfo == NULL)
> !     return;
>     memset (_heapinfo, 0, heapsize * sizeof (malloc_info));
>     _heapinfo[0].free.size = 0;
>     _heapinfo[0].free.next = _heapinfo[0].free.prev = 0;
> *************** __malloc_initialize ()
> *** 565,571 ****
>   
>     __malloc_initialized = 1;
>     PROTECT_MALLOC_STATE (1);
> !   return 1;
>   }
>   
>   static int morecore_recursing;
> --- 587,609 ----
>   
>     __malloc_initialized = 1;
>     PROTECT_MALLOC_STATE (1);
> !   return;
> ! }
> ! 
> ! /* Set everything up and remember that we have.  */
> ! int
> ! __malloc_initialize ()
> ! {
> ! #ifdef USE_PTHREAD
> !   pthread_once (&malloc_init_once_control, malloc_initialize_1);
> ! #else
> !   if (__malloc_initialized)
> !     return 0;
> ! 
> !   malloc_initialize_1 ();
> ! #endif
> ! 
> !   return __malloc_initialized;
>   }
>   
>   static int morecore_recursing;
> *************** _malloc_internal (size)
> *** 708,713 ****
> --- 746,754 ----
>       return NULL;
>   #endif
>   
> + #ifdef USE_PTHREAD
> +   pthread_mutex_lock (&_malloc_mutex);
> + #endif
>     PROTECT_MALLOC_STATE (0);
>   
>     if (size < sizeof (struct list))
> *************** _malloc_internal (size)
> *** 765,771 ****
>   	  if (result == NULL)
>   	    {
>   	      PROTECT_MALLOC_STATE (1);
> ! 	      return NULL;
>   	    }
>   
>   	  /* Link all fragments but the first into the free list.  */
> --- 806,812 ----
>   	  if (result == NULL)
>   	    {
>   	      PROTECT_MALLOC_STATE (1);
> ! 	      goto out;
>   	    }
>   
>   	  /* Link all fragments but the first into the free list.  */
> *************** _malloc_internal (size)
> *** 831,837 ****
>   		}
>   	      result = morecore (wantblocks * BLOCKSIZE);
>   	      if (result == NULL)
> ! 		return NULL;
>   	      block = BLOCK (result);
>   	      /* Put the new block at the end of the free list.  */
>   	      _heapinfo[block].free.size = wantblocks;
> --- 872,878 ----
>   		}
>   	      result = morecore (wantblocks * BLOCKSIZE);
>   	      if (result == NULL)
> ! 		goto out;
>   	      block = BLOCK (result);
>   	      /* Put the new block at the end of the free list.  */
>   	      _heapinfo[block].free.size = wantblocks;
> *************** _malloc_internal (size)
> *** 886,891 ****
> --- 927,936 ----
>       }
>   
>     PROTECT_MALLOC_STATE (1);
> +  out:
> + #ifdef USE_PTHREAD
> +   pthread_mutex_unlock (&_malloc_mutex);
> + #endif
>     return result;
>   }
>   
> *************** _free_internal (ptr)
> *** 996,1001 ****
> --- 1041,1049 ----
>     if (ptr == NULL)
>       return;
>   
> + #ifdef USE_PTHREAD
> +   pthread_mutex_lock (&_malloc_mutex);
> + #endif
>     PROTECT_MALLOC_STATE (0);
>   
>     for (l = _aligned_blocks; l != NULL; l = l->next)
> *************** _free_internal (ptr)
> *** 1221,1226 ****
> --- 1269,1277 ----
>       }
>   
>     PROTECT_MALLOC_STATE (1);
> + #ifdef USE_PTHREAD
> +   pthread_mutex_unlock (&_malloc_mutex);
> + #endif
>   }
>   
>   /* Return memory to the heap.  */
> *************** _realloc_internal (ptr, size)
> *** 1384,1389 ****
> --- 1435,1443 ----
>   
>     block = BLOCK (ptr);
>   
> + #ifdef USE_PTHREAD
> +   pthread_mutex_lock (&_malloc_mutex);
> + #endif
>     PROTECT_MALLOC_STATE (0);
>   
>     type = _heapinfo[block].busy.type;
> *************** _realloc_internal (ptr, size)
> *** 1398,1404 ****
>   	    {
>   	      memcpy (result, ptr, size);
>   	      _free_internal (ptr);
> ! 	      return result;
>   	    }
>   	}
>   
> --- 1452,1458 ----
>   	    {
>   	      memcpy (result, ptr, size);
>   	      _free_internal (ptr);
> ! 	      goto out;
>   	    }
>   	}
>   
> *************** _realloc_internal (ptr, size)
> *** 1451,1457 ****
>   		  (void) _malloc_internal (blocks * BLOCKSIZE);
>   		  _free_internal (previous);
>   		}
> ! 	      return NULL;
>   	    }
>   	  if (ptr != result)
>   	    memmove (result, ptr, blocks * BLOCKSIZE);
> --- 1505,1511 ----
>   		  (void) _malloc_internal (blocks * BLOCKSIZE);
>   		  _free_internal (previous);
>   		}
> ! 	      goto out;
>   	    }
>   	  if (ptr != result)
>   	    memmove (result, ptr, blocks * BLOCKSIZE);
> *************** _realloc_internal (ptr, size)
> *** 1471,1477 ****
>   	     and copy the lesser of the new size and the old. */
>   	  result = _malloc_internal (size);
>   	  if (result == NULL)
> ! 	    return NULL;
>   	  memcpy (result, ptr, min (size, (__malloc_size_t) 1 << type));
>   	  _free_internal (ptr);
>   	}
> --- 1525,1531 ----
>   	     and copy the lesser of the new size and the old. */
>   	  result = _malloc_internal (size);
>   	  if (result == NULL)
> ! 	    goto out;
>   	  memcpy (result, ptr, min (size, (__malloc_size_t) 1 << type));
>   	  _free_internal (ptr);
>   	}
> *************** _realloc_internal (ptr, size)
> *** 1479,1484 ****
> --- 1533,1542 ----
>       }
>   
>     PROTECT_MALLOC_STATE (1);
> +  out:
> + #ifdef USE_PTHREAD
> +   pthread_mutex_unlock (&_malloc_mutex);
> + #endif
>     return result;
>   }
>   
> 
> 
> _______________________________________________
> Emacs-devel mailing list
> Emacs-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-devel
> 

Re: Pretest?

[Jan Djärv]

YAMAMOTO Mitsuharu skrev:
>>>>>> On Mon, 19 Mar 2007 09:00:10 +0100, Jan Djärv <jan.h.d@swipnet.se> said:
> 
>> A thing to do after the release might be to synchronize with the
>> thread safe malloc in glibc.
> 
> What do we do before the release?  Possible options would be:
> 
>   1) Use the system malloc library (and abandon emacs_blocked_*) if
>      HAVE_GTK_AND_PTHREAD.
>   2) Modify src/gmalloc.c to make it thread safe.
> 
> I just tried the latter, but I can't test it myself.

I've been running this patch for a week now, and had no ill effects.

	Jan D.

Re: Pretest?

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 27 Mar 2007 10:07:27 +0200, Jan Djärv <jan.h.d@swipnet.se> said:

>> What do we do before the release?  Possible options would be:
>> 
>> 1) Use the system malloc library (and abandon emacs_blocked_*) if
>> HAVE_GTK_AND_PTHREAD.
>> 2) Modify src/gmalloc.c to make it thread safe.
>> 
>> I just tried the latter, but I can't test it myself.

> I've been running this patch for a week now, and had no ill effects.

Thanks for testing.  I installed the patch with the changes you
suggested.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Pretest?

[Jan Djärv]

Chong Yidong skrev:
> Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
> 
>>   FreeBSD 7.0-CURRENT #0: Tue Feb 27 01:25:46 EET 2007
>>
>> While I'm running the GTK+ version, however, I can crash Emacs in
>> emacs_blocked_free() by following the steps outlined below:
>>
>> * Run Emacs inside gdb:
>> * Run M-x gnus-agent-batch while my network connection is
>>   disabled, and let it time-out.  It prompts me for going into
>>   `off-line mode', to which I reply `yes'.
>> * The next time I input C-z Emacs crashes with a backtrace of:
> 
> I can't seem to reproduce this on GNU/Linux (I don't have a FreeBSD
> box handy).  It's strange that gnus-agent batch has anything to do
> with it.  Have you been able to reproduce the C-z crash in any other
> circumstance?  (It is better to get a recipe not involving gnus, since
> that might depend on your newsgroup settings.)
> 
> In any case, the backtrace indicates that the crash occurs deep in
> GTK/Glib.  If you look at what is occurring in the Emacs code, what
> we're doing is perfectly innocuous: gdk_pixbuf_new_from_xpm_data() is
> called on a static character array containing an XPM image.  So the
> bug is probably in GTK or Glib, not in Emacs.

It looks like the heap is corrupted, it may be something Emacs did previously, 
or perhaps something Gtk+ does.  Very hard to tell though, a pity valgrind 
doesn't work with Emacs.

	Jan D.

Re: Pretest?

[Piet van Oostrum]

I am running the 22.0.95 pretest on Mac OS X 10.4 on an Intel MacBook.
Installation went without problems. I still expect to get one of those
recursive malloc lockups. I had one with the 22.0.93 pretest, but gdb
wouldn't give a decent stack trace so I can't tell which system call caused
it. And it happened only once in 2-3 weeks, so the next one may take some
weeks to occur (but could also happen in the next hour).
-- 
Piet van Oostrum <piet@cs.uu.nl>
URL: http://www.cs.uu.nl/~piet [PGP 8DAE142BE17999C4]
Private email: piet@vanoostrum.org

Re: Pretest?

[Richard Stallman]

    I am running the 22.0.95 pretest on Mac OS X 10.4 on an Intel MacBook.
    Installation went without problems. I still expect to get one of those
    recursive malloc lockups. I had one with the 22.0.93 pretest, but gdb
    wouldn't give a decent stack trace so I can't tell which system call caused
    it.

Do you get a useful backtrace this time?

Re: Pretest?

[Piet van Oostrum]

>>>>> Richard Stallman <rms@gnu.org> (RS) wrote:

>RS>     I am running the 22.0.95 pretest on Mac OS X 10.4 on an Intel MacBook.
>RS>     Installation went without problems. I still expect to get one of those
>RS>     recursive malloc lockups. I had one with the 22.0.93 pretest, but gdb
>RS>     wouldn't give a decent stack trace so I can't tell which system call caused
>RS>     it.

>RS> Do you get a useful backtrace this time?

Sorry, I didn't mean I had a new lockup at that time. But as the problem
has not yet been identified it is to be expected that another one will
appear in a finite time.

However, yesterday I had another kind of lockup. I was trying to get new
email (I use VM for reading mail) and my emacs completely hung. It wouldn't
even react to C-g. Here is a backtrace:
This is Carbon Emacs 22.0.95 (pretest tarball) on Mac OS X 10.4 (Tiger)
installed as application bundle.

#0  0x9000fb57 in read ()
#1  0x000919d4 in emacs_read (fildes=18, buf=0xbffebc1c "??\f", nbyte=16384) at sysdep.c:3342
#2  0x001218a2 in Fcall_process (nargs=31, args=0xbfffc51c) at callproc.c:784
#3  0x001229bc in Fcall_process_region (nargs=33, args=0xbfffc51c) at callproc.c:1177
#4  0x000e5966 in Ffuncall (nargs=34, args=0xbfffc510) at eval.c:2978
#5  0x000e6f88 in Fapply (nargs=8, args=0xbfffc684) at eval.c:2485
#6  0x000e5966 in Ffuncall (nargs=9, args=0xbfffc680) at eval.c:2978
#7  0x00115f9a in Fbyte_code (bytestr=64064707, vector=51831380, maxdepth=19) at bytecode.c:679
#8  0x000e530b in funcall_lambda (fun=51831668, nargs=1, arg_vector=0xbfffc844) at eval.c:3184
#9  0x000e57cb in Ffuncall (nargs=2, args=0xbfffc840) at eval.c:3054
#10 0x00115f9a in Fbyte_code (bytestr=64239843, vector=51831732, maxdepth=2) at bytecode.c:679
#11 0x000e530b in funcall_lambda (fun=51831892, nargs=1, arg_vector=0xbfffc9c4) at eval.c:3184
#12 0x000e57cb in Ffuncall (nargs=2, args=0xbfffc9c0) at eval.c:3054
#13 0x00115f9a in Fbyte_code (bytestr=65100387, vector=51864692, maxdepth=3) at bytecode.c:679
#14 0x000e530b in funcall_lambda (fun=51864932, nargs=2, arg_vector=0xbfffcad0) at eval.c:3184
#15 0x000e5563 in apply_lambda (fun=51864932, args=16719909, eval_flag=1) at eval.c:3108
#16 0x000e4ccd in Feval (form=16719917) at eval.c:2388
#17 0x000e5232 in Fprogn (args=16720213) at eval.c:447
#18 0x000e770c in Flet (args=16720221) at eval.c:1064
#19 0x000e4f38 in Feval (form=16720229) at eval.c:2275
#20 0x000e7429 in internal_lisp_condition_case (var=43171641, bodyform=16720229, handlers=16720421) at eval.c:1426
#21 0x000e74ad in Fcondition_case (args=16720437) at eval.c:1367
#22 0x000e4f38 in Feval (form=16720445) at eval.c:2275
#23 0x000e5232 in Fprogn (args=16720461) at eval.c:447
#24 0x000e4f38 in Feval (form=16720469) at eval.c:2275
#25 0x000e5214 in Fprogn (args=16720485) at eval.c:447
#26 0x000e770c in Flet (args=16720493) at eval.c:1064
#27 0x000e4f38 in Feval (form=16720501) at eval.c:2275
#28 0x000e5232 in Fprogn (args=16720517) at eval.c:447
#29 0x000e549f in funcall_lambda (fun=16720528, nargs=1, arg_vector=0xbfffd174) at eval.c:3177
#30 0x000e57cb in Ffuncall (nargs=2, args=0xbfffd170) at eval.c:3054
#31 0x00115f9a in Fbyte_code (bytestr=62488915, vector=39195924, maxdepth=3) at bytecode.c:679
#32 0x000e4ed6 in Feval (form=16377813) at eval.c:2338
#33 0x000e7429 in internal_lisp_condition_case (var=41944073, bodyform=16377813, handlers=16379949) at eval.c:1426
#34 0x00114ed0 in Fbyte_code (bytestr=62488659, vector=39196148, maxdepth=7) at bytecode.c:869
#35 0x000e530b in funcall_lambda (fun=39196500, nargs=1, arg_vector=0xbfffd524) at eval.c:3184
#36 0x000e57cb in Ffuncall (nargs=2, args=0xbfffd520) at eval.c:3054
#37 0x00115f9a in Fbyte_code (bytestr=62168963, vector=39200612, maxdepth=8) at bytecode.c:679
#38 0x000e530b in funcall_lambda (fun=39200820, nargs=1, arg_vector=0xbfffd6b4) at eval.c:3184
#39 0x000e57cb in Ffuncall (nargs=2, args=0xbfffd6b0) at eval.c:3054
#40 0x00115f9a in Fbyte_code (bytestr=62168931, vector=39200420, maxdepth=2) at bytecode.c:679
#41 0x000e530b in funcall_lambda (fun=39200548, nargs=1, arg_vector=0xbfffd834) at eval.c:3184
#42 0x000e57cb in Ffuncall (nargs=2, args=0xbfffd830) at eval.c:3054
#43 0x00115f9a in Fbyte_code (bytestr=62488915, vector=39195924, maxdepth=3) at bytecode.c:679
#44 0x000e4ed6 in Feval (form=16377813) at eval.c:2338
#45 0x000e7429 in internal_lisp_condition_case (var=41944073, bodyform=16377813, handlers=16379949) at eval.c:1426
#46 0x00114ed0 in Fbyte_code (bytestr=62488659, vector=39196148, maxdepth=7) at bytecode.c:869
#47 0x000e530b in funcall_lambda (fun=39196500, nargs=1, arg_vector=0xbfffdbe4) at eval.c:3184
#48 0x000e57cb in Ffuncall (nargs=2, args=0xbfffdbe0) at eval.c:3054
#49 0x00115f9a in Fbyte_code (bytestr=64996067, vector=51897956, maxdepth=4) at bytecode.c:679
#50 0x000e530b in funcall_lambda (fun=51898100, nargs=1, arg_vector=0xbfffdd64) at eval.c:3184
#51 0x000e57cb in Ffuncall (nargs=2, args=0xbfffdd60) at eval.c:3054
#52 0x00115f9a in Fbyte_code (bytestr=62488915, vector=39195924, maxdepth=3) at bytecode.c:679
#53 0x000e4ed6 in Feval (form=16377813) at eval.c:2338
#54 0x000e7429 in internal_lisp_condition_case (var=41944073, bodyform=16377813, handlers=16379949) at eval.c:1426
#55 0x00114ed0 in Fbyte_code (bytestr=62488659, vector=39196148, maxdepth=7) at bytecode.c:869
#56 0x000e530b in funcall_lambda (fun=39196500, nargs=1, arg_vector=0xbfffe114) at eval.c:3184
#57 0x000e57cb in Ffuncall (nargs=2, args=0xbfffe110) at eval.c:3054
#58 0x00115f9a in Fbyte_code (bytestr=62488387, vector=39195316, maxdepth=6) at bytecode.c:679
#59 0x000e530b in funcall_lambda (fun=39195700, nargs=0, arg_vector=0xbfffe230) at eval.c:3184
#60 0x000e5563 in apply_lambda (fun=39195700, args=41944073, eval_flag=1) at eval.c:3108
#61 0x000e4ccd in Feval (form=16426621) at eval.c:2388
#62 0x000e509c in Fsetq (args=16426605) at eval.c:549
#63 0x000e4f38 in Feval (form=16426597) at eval.c:2275
#64 0x000e5214 in Fprogn (args=16426589) at eval.c:447
#65 0x000e770c in Flet (args=16426581) at eval.c:1064
#66 0x000e4f38 in Feval (form=16426573) at eval.c:2275
#67 0x000e5214 in Fprogn (args=16426557) at eval.c:447
#68 0x000e4f38 in Feval (form=16426533) at eval.c:2275
#69 0x000e5214 in Fprogn (args=16426485) at eval.c:447
#70 0x000e4f38 in Feval (form=16426477) at eval.c:2275
#71 0x000e5214 in Fprogn (args=16426461) at eval.c:447
#72 0x000e770c in Flet (args=16426357) at eval.c:1064
#73 0x000e4f38 in Feval (form=16426349) at eval.c:2275
#74 0x000e5214 in Fprogn (args=16426325) at eval.c:447
#75 0x000e549f in funcall_lambda (fun=16426304, nargs=0, arg_vector=0xbfffe850) at eval.c:3177
#76 0x000e5563 in apply_lambda (fun=16426309, args=41944073, eval_flag=1) at eval.c:3108
#77 0x000e4ccd in Feval (form=16326021) at eval.c:2388
#78 0x000e7429 in internal_lisp_condition_case (var=42150009, bodyform=16326021, handlers=16326093) at eval.c:1426
#79 0x00114ed0 in Fbyte_code (bytestr=62555299, vector=39173012, maxdepth=6) at bytecode.c:869
#80 0x000e530b in funcall_lambda (fun=39173316, nargs=0, arg_vector=0xbfffeb94) at eval.c:3184
#81 0x000e57cb in Ffuncall (nargs=1, args=0xbfffeb90) at eval.c:3054
#82 0x00115f9a in Fbyte_code (bytestr=62555603, vector=39172420, maxdepth=5) at bytecode.c:679
#83 0x000e530b in funcall_lambda (fun=39172804, nargs=0, arg_vector=0xbfffecb0) at eval.c:3184
#84 0x000e5563 in apply_lambda (fun=39172804, args=41944073, eval_flag=1) at eval.c:3108
#85 0x000e4ccd in Feval (form=16417749) at eval.c:2388
#86 0x000e509c in Fsetq (args=16417733) at eval.c:549
#87 0x000e4f38 in Feval (form=16417725) at eval.c:2275
#88 0x000e5214 in Fprogn (args=16416717) at eval.c:447
#89 0x000e770c in Flet (args=16416709) at eval.c:1064
#90 0x000e4f38 in Feval (form=16416701) at eval.c:2275
#91 0x000e5232 in Fprogn (args=16416677) at eval.c:447
#92 0x000e549f in funcall_lambda (fun=16416632, nargs=0, arg_vector=0xbffff094) at eval.c:3177
#93 0x000e57cb in Ffuncall (nargs=1, args=0xbffff090) at eval.c:3054
#94 0x00115f9a in Fbyte_code (bytestr=65081523, vector=51933540, maxdepth=6) at bytecode.c:679
#95 0x000e530b in funcall_lambda (fun=51979492, nargs=1, arg_vector=0xbffff274) at eval.c:3184
#96 0x000e57cb in Ffuncall (nargs=2, args=0xbffff270) at eval.c:3054
#97 0x000e2a53 in Fcall_interactively (function=62358425, record_flag=41944073, keys=34606876) at callint.c:860
#98 0x0007e309 in Fcommand_execute (cmd=62358425, record_flag=41944073, keys=41944073, special=41944073) at keyboard.c:10014
#99 0x00086083 in command_loop_1 () at keyboard.c:1873
#100 0x000e3c2a in internal_condition_case (bfun=0x85ccc <command_loop_1>, handlers=41990313, hfun=0x7ef88 <cmd_error>) at eval.c:1481
#101 0x0007883d in command_loop_2 () at keyboard.c:1329
#102 0x000e3b1b in internal_catch (tag=41986593, func=0x787f9 <command_loop_2>, arg=41944073) at eval.c:1222
#103 0x00078617 in command_loop () at keyboard.c:1308
#104 0x000786cb in recursive_edit_1 () at keyboard.c:1006
#105 0x000787ba in Frecursive_edit () at keyboard.c:1067
#106 0x0007799f in main (argc=2, argv=0xbffff8e0) at emacs.c:1761

-- 
Piet van Oostrum <piet@cs.uu.nl>
URL: http://www.cs.uu.nl/~piet [PGP 8DAE142BE17999C4]
Private email: piet@vanoostrum.org

Re: Pretest?

[YAMAMOTO Mitsuharu]

>>>>> On Wed, 14 Mar 2007 08:21:20 +0100, Piet van Oostrum <piet@cs.uu.nl> said:

>>>>> Richard Stallman <rms@gnu.org> (RS) wrote:
RS> Do you get a useful backtrace this time?

> Sorry, I didn't mean I had a new lockup at that time. But as the
> problem has not yet been identified it is to be expected that
> another one will appear in a finite time.

In principle, that can happen.  As I said previously (*1), there are
still some library functions that may call malloc-related functions
internally but their calls are not pretected by BLOCK_INPUT:

    getc, ungetc, fwrite, fclose,
    getaddrinfo, freeaddrinfo

(*1) http://lists.gnu.org/archive/html/emacs-devel/2007-01/msg00125.html

> However, yesterday I had another kind of lockup. I was trying to get
> new email (I use VM for reading mail) and my emacs completely
> hung. It wouldn't even react to C-g.

You cannot quit with C-g if inhibit-quit is non-nil (*2).  Can you
check the values of Vinhibit_quit and Qnil if you still have the
debugging session?

(*2) http://lists.gnu.org/archive/html/emacs-pretest-bug/2006-06/msg00320.html

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Pretest?

[Richard Stallman]

	getc, ungetc, fwrite, fclose,
	getaddrinfo, freeaddrinfo

They are not used in very many places.  Would you like
to add BLOCK_INPUT for them?

Re: Pretest?

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 15 Mar 2007 01:31:00 -0400, Richard Stallman <rms@gnu.org> said:

> 	getc, ungetc, fwrite, fclose,
> 	getaddrinfo, freeaddrinfo

> They are not used in very many places.  Would you like to add
> BLOCK_INPUT for them?

Yes, but a couple of things are not clear to me:

  1. I think BLOCK_INPUT is not necessary when `noninteractive' is
     set.  Is that correct?

  2. getaddrinfo/freeaddrinfo are called with immediate_quit == 1.  We
     have to choose from either protecting them with BLOCK_INPUT or
     allowing quit during their executions, especially on the systems
     where SYSTEM_MALLOC is defined and thus emacs_blocked_malloc
     etc. cannot be used.

				    YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Pretest?

[Richard Stallman]

      1. I think BLOCK_INPUT is not necessary when `noninteractive' is
	 set.  Is that correct?

I think so, because Emacs does not use signals for keyboard input
in batch mode.

However, I am not sure whether handling of other signals (such as SIGCHLD)
calls for BLOCK_INPUT nowadays.  If it does, then BLOCK_INPUT should be
needed also in batch mode, because subprocess can be used.

So you may as well do BLOCK_INPUT for getc, etc.

      2. getaddrinfo/freeaddrinfo are called with immediate_quit == 1.  We
	 have to choose from either protecting them with BLOCK_INPUT or
	 allowing quit during their executions, especially on the systems
	 where SYSTEM_MALLOC is defined and thus emacs_blocked_malloc
	 etc. cannot be used.

I am not familiar with getaddrinfo.  Why do users want to be
able to interrupt it?  Is that because it communicates with
a DNS server?

The only way I can think of to make this work right
is to run it in another thread, or use child labor.

Re: Pretest?

[YAMAMOTO Mitsuharu]

>>>>> On Fri, 16 Mar 2007 01:20:27 -0400, Richard Stallman <rms@gnu.org> said:

> However, I am not sure whether handling of other signals (such as
> SIGCHLD) calls for BLOCK_INPUT nowadays.  If it does, then
> BLOCK_INPUT should be needed also in batch mode, because subprocess
> can be used.

BLOCK_INPUT just increments the variable interrupt_input_blocked, and
it is currently only examined by the SIGALRM/SIGIO handler.

> I am not familiar with getaddrinfo.  Why do users want to be able to
> interrupt it?  Is that because it communicates with a DNS server?

I'm not certain about that, but I guess the assignments to
immediate_quit were copied from those around gethostbyname when the
getaddrinfo support was added.

> The only way I can think of to make this work right is to run it in
> another thread, or use child labor.

Yes, but I think neither of them is a thing to do now.  I feel a bit
inclined to put BLOCK_INPUT around getaddrinfo/freeaddrinfo to be on
the safe side.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Pretest?

[Kim F. Storm]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

>> I am not familiar with getaddrinfo.  Why do users want to be able to
>> interrupt it?  Is that because it communicates with a DNS server?
>
> I'm not certain about that, but I guess the assignments to
> immediate_quit were copied from those around gethostbyname when the
> getaddrinfo support was added.

At least gethostbyname can potentially block for 3 minutes (waiting
for DNS responses) - so I assumed getaddrinfo could do the same.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Pretest?

[Richard Stallman]

    Yes, but I think neither of them is a thing to do now.  I feel a bit
    inclined to put BLOCK_INPUT around getaddrinfo/freeaddrinfo to be on
    the safe side.

I would rather not.  You see, the chance that a quit during this call
will cause memory clobberage is very small.  It might happen once
a year.

But it will surely happen every day that users will get stuck in this
call, because they can't reach the DNS server.  We do not want to make
them wait 3 minutes for it to time out.

Re: Pretest?

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 19 Mar 2007 17:57:19 -0400, Richard Stallman <rms@gnu.org> said:

> I would rather not.  You see, the chance that a quit during this
> call will cause memory clobberage is very small.  It might happen
> once a year.

> But it will surely happen every day that users will get stuck in
> this call, because they can't reach the DNS server.  We do not want
> to make them wait 3 minutes for it to time out.

OK, I added BLOCK_INPUTs except for getaddrinfo.  I think this issue
will be revisited when we switch to SYNC_INPUT.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: Pretest?

[Richard Stallman]

    However, yesterday I had another kind of lockup. I was trying to get new
    email (I use VM for reading mail) and my emacs completely hung. It wouldn't
    even react to C-g. Here is a backtrace:
    This is Carbon Emacs 22.0.95 (pretest tarball) on Mac OS X 10.4 (Tiger)
    installed as application bundle.

Was it hung, or was it looping?

If it was looping, or if you're not sure, please
try the instructions in etc/DEBUG regarding what to do 
when Emacs is looping.

Re: cygwin succesfull straight build

[Eli Zaretskii]

> Date: Fri, 23 Feb 2007 14:07:19 +0100 (MET)
> From: Angelo Graziosi <Angelo.Graziosi@roma1.infn.it>
> cc: emacs-devel@gnu.org
> 
> Eli Zaretskii wrote:
> 
> > It is important for us to know whether the distribution tarball builds
> > (./configure; make; make install) cleanly on all supported platforms.
> 
> 
> If the pretest is
> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.93.tar.gz (timestamp
> 01/23/2007 01:16, size 37,118,034), I have built it on Cygwin, since it
> was released, without problems (using GCC-4.0.3 for stability reasons).

Thanks.  But I'd like to hear from other Cygwin users as well, if
there are any.

Re: Pretest?

[Angelo Graziosi]

Chong Yidong wrote:

> I have rolled a 22.0.95 tarball, which can be found at the usual
> location:

> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.95.tar.gz
> ...
>

I would know, if possible, how emacs-22.0.95.tar.gz has been generated.


Thanks,

   Angelo.

Re: Pretest?

[Chong Yidong]

Angelo Graziosi <Angelo.Graziosi@roma1.infn.it> writes:

> Chong Yidong wrote:
>
>> I have rolled a 22.0.95 tarball, which can be found at the usual
>> location:
>
>> ftp://alpha.gnu.org/gnu/emacs/pretest/emacs-22.0.95.tar.gz
>> ...
>>
>
> I would know, if possible, how emacs-22.0.95.tar.gz has been generated.

See admin/make-tarball.txt in the CVS repository.

Re: Pretest?

[Giorgos Keramidas]

On 2007-03-08 14:40, Chong Yidong <cyd@stupidchicken.com> wrote:
>> I would know, if possible, how emacs-22.0.95.tar.gz has been generated.
> 
> See admin/make-tarball.txt in the CVS repository.

Hi all, just a quick question:

Are we going to have a new pretest tarball this week too?

Re: Pretest?

[Chong Yidong]

Giorgos Keramidas <keramida@ceid.upatras.gr> writes:

> On 2007-03-08 14:40, Chong Yidong <cyd@stupidchicken.com> wrote:
>>> I would know, if possible, how emacs-22.0.95.tar.gz has been generated.
>> 
>> See admin/make-tarball.txt in the CVS repository.
>
> Hi all, just a quick question:
>
> Are we going to have a new pretest tarball this week too?

I would like to suggest issuing a pretest on Monday the 12th.  What do
people think?

Re: Pretest?

[Christian Faulhammer]

[-- Attachment #1.1: Type: text/plain, Size: 299 bytes --]

Chong Yidong <cyd@stupidchicken.com>:

> > Are we going to have a new pretest tarball this week too?  
> I would like to suggest issuing a pretest on Monday the 12th.  What do
> people think?

 Good idea, as there is a at least one annyoing bug in the snapshot
that is fixed in CVS.

V-Li

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: Pretest?

[Juanma Barranquero]

On 3/9/07, Christian Faulhammer <v-li@gmx.de> wrote:

>  Good idea, as there is a at least one annyoing bug in the snapshot
> that is fixed in CVS.

What about

  M-: (describe-buffer-bindings "*scratch*")  =>  CRASH!

Is not annoying, just un-nice.

             Juanma

Re: Pretest?

[Chong Yidong]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/9/07, Christian Faulhammer <v-li@gmx.de> wrote:
>
>>  Good idea, as there is a at least one annyoing bug in the snapshot
>> that is fixed in CVS.
>
> What about
>
>  M-: (describe-buffer-bindings "*scratch*")  =>  CRASH!
>
> Is not annoying, just un-nice.

Thanks for finding that bug (and fixing it).

Re: Pretest?

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Fri, 09 Mar 2007 09:44:23 -0500
> 
> I would like to suggest issuing a pretest on Monday the 12th.  What do
> people think?

I think we need a new pretest ASAP, as the current one has a broken
ps-print.  March 12 is fine.

Re: Pretest?

[Giorgos Keramidas]

On 2007-03-09 19:49, Eli Zaretskii <eliz@gnu.org> wrote:
> > Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> > From: Chong Yidong <cyd@stupidchicken.com>
> > Date: Fri, 09 Mar 2007 09:44:23 -0500
> > 
> > I would like to suggest issuing a pretest on Monday the 12th.  What do
> > people think?
> 
> I think we need a new pretest ASAP, as the current one has a broken
> ps-print.  March 12 is fine.

Cool!  This was partly the reason for my asking.  emacs-22.0.95 and
ps-print fails here, but CVS head works fine.

Thanks for the fast replies :)

Re: Pretest?

[Richard Stallman]

    I would like to suggest issuing a pretest on Monday the 12th.  What do
    people think?

Sounds good to me.

Re: Pretest?

[Juanma Barranquero]

On 3/9/07, Chong Yidong <cyd@stupidchicken.com> wrote:

> I would like to suggest issuing a pretest on Monday the 12th.  What do
> people think?

If you don't mind, I'd like to reach a consensus with respect to the
emacsclient / isearch discussion in emacs-pretest-bugs. The proposed
patch is tiny (seven lines plus comments), but the issue is larger, as
it seems like Richard doesn't like emacsclient / server "forcibly
exit[ing] the minibuffer" or "interfer[ing] with isearch".

             Juanma

Re: Pretest?

[David Kastrup]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/9/07, Chong Yidong <cyd@stupidchicken.com> wrote:
>
>> I would like to suggest issuing a pretest on Monday the 12th.  What do
>> people think?
>
> If you don't mind, I'd like to reach a consensus with respect to the
> emacsclient / isearch discussion in emacs-pretest-bugs. The proposed
> patch is tiny (seven lines plus comments), but the issue is larger, as
> it seems like Richard doesn't like emacsclient / server "forcibly
> exit[ing] the minibuffer" or "interfer[ing] with isearch".

Well, seems like diverting the pretest-bug list to emacs-devel might
not be the worst idea after all.

-- 
David Kastrup

Re: Pretest?

[Juanma Barranquero]

On 3/12/07, David Kastrup <dak@gnu.org> wrote:
> "Juanma Barranquero" <lekktu@gmail.com> writes:

> Well, seems like diverting the pretest-bug list to emacs-devel might
> not be the worst idea after all.

My thoughs exactly ;-)

             Juanma

Re: Pretest?

[Stefan Monnier]

> If you don't mind, I'd like to reach a consensus with respect to the
> emacsclient / isearch discussion in emacs-pretest-bugs.

I don't see why this should delay the next pretest.


        Stefan

Re: Pretest?

[Juanma Barranquero]

On 3/12/07, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I don't see why this should delay the next pretest.

It seems a bit silly to release a pretest just to change something
non-trivial immediately afterwards. The patch I'm proposing is
trivial, but IIUC what Richard is saying, perhaps greater changes will
be needed. There's no harm in waiting a day or three, is it?

             Juanma

Re: Pretest?

[David Kastrup]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/12/07, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
>> I don't see why this should delay the next pretest.
>
> It seems a bit silly to release a pretest just to change something
> non-trivial immediately afterwards. The patch I'm proposing is
> trivial, but IIUC what Richard is saying, perhaps greater changes will
> be needed. There's no harm in waiting a day or three, is it?

For every pretest, I hope it will be the last.  My next important
conference with Emacs tutorials is at the end of April.  A release
will make persuading people to use Emacs 22 much easier.

So I agree that it seems pointless to make a prerelease when
fundamental things are scheduled for change or fixes.

-- 
David Kastrup

Re: Pretest?

[Chong Yidong]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/9/07, Chong Yidong <cyd@stupidchicken.com> wrote:
>
>> I would like to suggest issuing a pretest on Monday the 12th.  What do
>> people think?
>
> If you don't mind, I'd like to reach a consensus with respect to the
> emacsclient / isearch discussion in emacs-pretest-bugs. The proposed
> patch is tiny (seven lines plus comments), but the issue is larger, as
> it seems like Richard doesn't like emacsclient / server "forcibly
> exit[ing] the minibuffer" or "interfer[ing] with isearch".

>From reading the discussion, I don't think there is any indication
that a big change to Emacs is being called for (even taking Richard's
rather terse comments into account).

Re: Pretest?

[Juanma Barranquero]

On 3/12/07, Chong Yidong <cyd@stupidchicken.com> wrote:

> From reading the discussion, I don't think there is any indication
> that a big change to Emacs is being called for

I didn't say "big". Still, I think significant would be a good description.

             Juanma

Re: Pretest?

[Chong Yidong]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/12/07, Chong Yidong <cyd@stupidchicken.com> wrote:
>
>> From reading the discussion, I don't think there is any indication
>> that a big change to Emacs is being called for
>
> I didn't say "big". Still, I think significant would be a good description.

OK, let's wait a couple of days till Wednesday the 14th.

I'd like to know exactly what "significant" changes Richard is asking
for.

Re: Pretest?

[Juanma Barranquero]

On 3/13/07, Chong Yidong <cyd@stupidchicken.com> wrote:
> I'd like to know exactly what "significant" changes Richard is asking
> for.

The ones in the next message in the thread?

             Juanma

Re: Pretest?

[Richard Stallman]

    If you don't mind, I'd like to reach a consensus with respect to the
    emacsclient / isearch discussion in emacs-pretest-bugs. The proposed
    patch is tiny (seven lines plus comments), but the issue is larger, as
    it seems like Richard doesn't like emacsclient / server "forcibly
    exit[ing] the minibuffer" or "interfer[ing] with isearch".

I think that in cases like this server.el should display the new buffer
in another window, so that the user knows to go over and edit it.
But server.el should not mess up the command that is being executed.

Re: Pretest?

[Juanma Barranquero]

On 3/13/07, Richard Stallman <rms@gnu.org> wrote:

> I think that in cases like this server.el should display the new buffer
> in another window, so that the user knows to go over and edit it.

If the user wants server.el to display emacsclient-sent buffers in
another window or frame, he can set server-window.

> But server.el should not mess up the command that is being executed.

As David pointed out, emacsclient is rarely started in an asynchronous
way; usually is as a result of user interaction, either clicking
somewhere or typing "emacsclient myfile" in a shell. "Messing up" with
the command being executed is exactly what I would expect from such an
action. And IIRC, both the current isearch issue and the previous
patch to abort recursive editing were prompted by users considering
not doing that as a bug.

             Juanma

Re: Pretest?

[Andreas Schwab]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/13/07, Richard Stallman <rms@gnu.org> wrote:
>
>> But server.el should not mess up the command that is being executed.
>
> As David pointed out, emacsclient is rarely started in an asynchronous
> way; usually is as a result of user interaction, either clicking
> somewhere or typing "emacsclient myfile" in a shell. "Messing up" with
> the command being executed is exactly what I would expect from such an
> action.

I agree.  I have sometimes wondered why my emacsclient frame does not pop
up (I have set server-window to create a new frame) when I have forgotten
to exit an isearch in another frame.  Normally almost any command will
abort the isearch.

> And IIRC, both the current isearch issue and the previous
> patch to abort recursive editing were prompted by users considering
> not doing that as a bug.

Which I agree with aborting isearch, I don't agree with aborting a
recursive editing, since the latter is not as much a modal thing.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Pretest?

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> "Juanma Barranquero" <lekktu@gmail.com> writes:
>
>> On 3/13/07, Richard Stallman <rms@gnu.org> wrote:
>>
>>> But server.el should not mess up the command that is being executed.
>>
>> As David pointed out, emacsclient is rarely started in an asynchronous
>> way; usually is as a result of user interaction, either clicking
>> somewhere or typing "emacsclient myfile" in a shell. "Messing up" with
>> the command being executed is exactly what I would expect from such an
>> action.
>
> I agree.  I have sometimes wondered why my emacsclient frame does not pop
> up (I have set server-window to create a new frame) when I have forgotten
> to exit an isearch in another frame.  Normally almost any command will
> abort the isearch.
>
>> And IIRC, both the current isearch issue and the previous
>> patch to abort recursive editing were prompted by users considering
>> not doing that as a bug.
>
> Which I agree with aborting isearch, I don't agree with aborting a
> recursive editing, since the latter is not as much a modal thing.

To make that clear: my suggestion was about exiting the minibuffer.
However, if point moves to the buffer window, exiting the minibuffer
can probably be left to the normal recursive-minibuffer mechanisms
that kick in when the user tries using the minibuffer other than
moving back into it with C-x o.

So that suggestion of mine was probably not really an improvement.

-- 
David Kastrup

Re: Pretest?

[Juanma Barranquero]

On 3/13/07, Andreas Schwab <schwab@suse.de> wrote:

> Which I agree with aborting isearch, I don't agree with aborting a
> recursive editing, since the latter is not as much a modal thing.

This is the relevant code from server.el, which has been in place for
107 days now:

  (when (> (recursion-depth) 0)
    ;; We're inside a minibuffer already, so if the emacs-client is trying
    ;; to open a frame on a new display, we might end up with an unusable
    ;; frame because input from that display will be blocked (until exiting
    ;; the minibuffer).  Better exit this minibuffer right away.
    ;; Similarly with recursive-edits such as the splash screen.
    (process-put proc :previous-string string)
    (run-with-timer 0 nil (lexical-let ((proc proc))
                            (lambda () (server-process-filter proc ""))))
    (top-level))

             Juanma

Re: Pretest?

[Richard Stallman]

	;; We're inside a minibuffer already, so if the emacs-client is trying
	;; to open a frame on a new display, we might end up with an unusable
	;; frame because input from that display will be blocked (until exiting
	;; the minibuffer).  Better exit this minibuffer right away.
	;; Similarly with recursive-edits such as the splash screen.

In the case where it really is on a different display, and if you
don't see that other display, the result would be very undesirable.
This code prevents that bad result.  Unfortunately, there is no way
ot distinguish the various cases, because there is no way for Lisp code
to find out which frame or display the minibuffer is on.

So I guess this has to be left alone.

Re: Pretest?

[Richard Stallman]

    > I think that in cases like this server.el should display the new buffer
    > in another window, so that the user knows to go over and edit it.

    If the user wants server.el to display emacsclient-sent buffers in
    another window or frame, he can set server-window.

Yes, but that is a different issue.  I am talking about what to do
when emacsclient wants to display a buffer, but something like a
minibuffer or an isearch prevents it from switching effectively to
that buffer in the normal way.

It should display that buffer in another window, and not abort
anything.

    And IIRC, both the current isearch issue and the previous
    patch to abort recursive editing were prompted by users considering
    not doing that as a bug.

I thought they complained because the buffer was not visible.

Re: Pretest?

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     > I think that in cases like this server.el should display the new buffer
>     > in another window, so that the user knows to go over and edit it.
>
>     If the user wants server.el to display emacsclient-sent buffers in
>     another window or frame, he can set server-window.
>
> Yes, but that is a different issue.  I am talking about what to do
> when emacsclient wants to display a buffer, but something like a
> minibuffer or an isearch prevents it from switching effectively to
> that buffer in the normal way.
>
> It should display that buffer in another window, and not abort
> anything.
>
>     And IIRC, both the current isearch issue and the previous
>     patch to abort recursive editing were prompted by users considering
>     not doing that as a bug.
>
> I thought they complained because the buffer was not visible.

You can't show the buffer if an isearch is going on in a single frame
with a single window and you are not going to interrupt isearch.

-- 
David Kastrup

Re: Pretest?

[Stefan Monnier]

> You can't show the buffer if an isearch is going on in a single frame
> with a single window and you are not going to interrupt isearch.

Why not?  Why can't it pop up in a new window?


        Stefan


PS: I don't know if it'd be a good idea, tho.  I'm just trying
    to understand.

Re: Pretest?

[David Kastrup]

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

>> You can't show the buffer if an isearch is going on in a single frame
>> with a single window and you are not going to interrupt isearch.
>
> Why not?  Why can't it pop up in a new window?

Where would that window be?  Are you talking about splitting the
frame?  That might involve recentering.  It would also be out of
character to move window-point to the new window, since isearch does
not normally offer a way to do window changes without exiting isearch.

-- 
David Kastrup

Re: Pretest?

[Stefan Monnier]

>>> You can't show the buffer if an isearch is going on in a single frame
>>> with a single window and you are not going to interrupt isearch.
>> Why not?  Why can't it pop up in a new window?
> Where would that window be?  Are you talking about splitting the
> frame?

Well, yes: a *new* window.

> That might involve recentering.

Big deal!

> It would also be out of character to move window-point to the new window,
> since isearch does not normally offer a way to do window changes without
> exiting isearch.

Well, as I said in my PS whether it's a good idea or not is a separate
issue.  I was asking specifically why you thought it "can't" be done.


        Stefan

Re: Pretest?

[Juanma Barranquero]

On 3/14/07, Richard Stallman <rms@gnu.org> wrote:

> I thought they complained because the buffer was not visible.

The reporter said: "emacsclient seems to interrupt find-file,
swith-to-buffer and areas marked to be copied, so I think it is
somewhat inconsistent not to quit isearch and put the new buffer on
top."

We haven't done a poll of user expectations with respect to
emacsclient, but I certainly wouldn't just expect the buffer to be
visible, but selected as well.

Cancelling isearch is not something done in extreme circunstances;
almost anything you type or do will cancel it (like switching to
another frame). What's so special in cancelling it from emacsclient?

             Juanma

Re: Pretest?

[David Kastrup]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/14/07, Richard Stallman <rms@gnu.org> wrote:
>
>> I thought they complained because the buffer was not visible.
>
> The reporter said: "emacsclient seems to interrupt find-file,
> swith-to-buffer and areas marked to be copied, so I think it is
> somewhat inconsistent not to quit isearch and put the new buffer on
> top."
>
> We haven't done a poll of user expectations with respect to
> emacsclient, but I certainly wouldn't just expect the buffer to be
> visible, but selected as well.
>
> Cancelling isearch is not something done in extreme circunstances;
> almost anything you type or do will cancel it (like switching to
> another frame). What's so special in cancelling it from emacsclient?

Agreed.  However, we should still come up with a suitable strategy
concerning what we should do when we are in a minibuffer for other
reasons (such as M-x).

My initial impulse would be to let an emacsclient call behave like C-x
C-f: this would with the default setting of
enable-recursive-minibuffers (nil) cancel the minibuffer command,
return to a non-minibuffer window and open the given file there.

With enable-recursive-minibuffers to t, however, would then cause the
message "Cannot switch buffers in minibuffer window".

Suboptimal.  But at least for the default settings, the behavior would
be very much like what is expected.

And I think even in the case where emacsclient -eval is used, the
assumption of "clear minibuffer usage" seems reasonable, since that is
somewhat equivalent to using M-:

-- 
David Kastrup

Re: Pretest?

[Juanma Barranquero]

On 3/14/07, David Kastrup <dak@gnu.org> wrote:

> However, we should still come up with a suitable strategy
> concerning what we should do when we are in a minibuffer for other
> reasons (such as M-x).

[...]

> With enable-recursive-minibuffers to t, however, would then cause the
> message "Cannot switch buffers in minibuffer window".

Curious. I have enable-recursive-minibuffers set to t, but I would
still expect emacsclient to interrupt M-x, etc.

             Juanma

Re: Pretest?

[David Kastrup]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On 3/14/07, David Kastrup <dak@gnu.org> wrote:
>
>> However, we should still come up with a suitable strategy
>> concerning what we should do when we are in a minibuffer for other
>> reasons (such as M-x).
>
> [...]
>
>> With enable-recursive-minibuffers to t, however, would then cause the
>> message "Cannot switch buffers in minibuffer window".
>
> Curious. I have enable-recursive-minibuffers set to t, but I would
> still expect emacsclient to interrupt M-x, etc.

What is "curious" about that?  I proposed what I consider a pretty
logical and consistent way of dealing with emacsclient calls, and then
mentioned a drawback of this for a certain setting.

And now you call it curious that what I mention as a drawback does not
strike you as the best behavior?

Curious.

Anyway: people are ready to forward their expectations all around.
The question is how we can tie the various expectations into some
rules and behavior that will be acceptable to most people under most
settings.

If we can find a simple rule set working for all cases reasonably
well, this would be optimal.

I proposed a simple rule set and mentioned that I considered some
special case of it undesirable.

Can you can come up with a good rule set (with as few exceptions as
possible, since we want to avoid introducing lots of special cases)
that is better-behaved?  If so, please do.

-- 
David Kastrup

Re: Pretest?

[Juanma Barranquero]

On 3/14/07, David Kastrup <dak@gnu.org> wrote:

> What is "curious" about that?  I proposed what I consider a pretty
> logical and consistent way of dealing with emacsclient calls, and then
> mentioned a drawback of this for a certain setting.
>
> And now you call it curious that what I mention as a drawback does not
> strike you as the best behavior?
>
> Curious.

Please, don't be so jumpy, because I tend to answer in the same mood
and that leads to me being labeled as "aggressive" pretty fast :)

My "curious" was meant as "it is curious how wildly different
expectations are from one user (or indeed, developer) to another". No
judgement, no implication that my choice was better, no nothing but a
little wonder about human variability (and irritability, I would add
as an afterthought).

Honestly: I just read an old thread from September 2005 where some
people was hopeful that 22.1 could be released that same year. So I'm
now just a little sad, and the issue of whether emacsclient interrupts
or not seems less and less important to me. I'd only like to ask that
an option be left somewhere so overeager interrupters like me can do
our thing without having to hack server.el.

             Juanma

Re: Pretest?

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     > I think that in cases like this server.el should display the new buffer
>     > in another window, so that the user knows to go over and edit it.
>
>     If the user wants server.el to display emacsclient-sent buffers in
>     another window or frame, he can set server-window.
>
> Yes, but that is a different issue.  I am talking about what to do
> when emacsclient wants to display a buffer, but something like a
> minibuffer or an isearch prevents it from switching effectively to
> that buffer in the normal way.
>
> It should display that buffer in another window, and not abort
> anything.

That might means the new buffer will not be immediately available for
editing, until the user cancels the minibuffer or isearch.  If the
user calls emacsclient, the intention is almost certainly to perform
the edit in Emacs, so this is a minor nuisance.

Another possible complication (which I haven't thought much about) is
when Emacs has frames open on more than one display.  If you are
currently working on one display, having left isearch on in another
display, invoking emacsclient might lead to puzzling behavior.

Anyway, I don't feel strongly about this issue one way or another.  As
Juanma has said, it's rather sad to have the Emacs 22 release drag on
and on while we deal with this kind of nitpicky issues.

Re: Pretest?

[Stefan Monnier]

> Another possible complication (which I haven't thought much about) is
> when Emacs has frames open on more than one display.  If you are
> currently working on one display, having left isearch on in another
> display, invoking emacsclient might lead to puzzling behavior.

Read the relevant code in server.el.  It contains a comment that explains
that the exit-minibuffer behavior is there specifically to deal with
multi-display situations.  This same problem doesn't directly affect isearch
I believe, or at least not to the same extent (with recursive edits, the
active terminal grabs control, so the other display's events are completely
blocked).

Supposedly, since isearch uses overriding-terminal-local-map, it should be
possible to keep isearch running on the other display, but I believe that
the current implementation of isearch mixes up terminal-local, buffer-local,
and global settings in such a messed up way that it won't work without
further changes.


        Stefan

Re: Pretest?

[Richard Stallman]

    > It should display that buffer in another window, and not abort
    > anything.

    That might means the new buffer will not be immediately available for
    editing, until the user cancels the minibuffer or isearch.

That is right.  The user will see the other buffer appear, and can
either finish or cancel the command that is in progress.  I think that
is less harsh.

Re: Pretest?

[Juanma Barranquero]

On 3/15/07, Richard Stallman <rms@gnu.org> wrote:

> That is right.  The user will see the other buffer appear, and can
> either finish or cancel the command that is in progress.  I think that
> is less harsh.

For some definition of "less harsh" that maps nicely to your tastes,
perhaps. I much prefer to be interrupted, and profoundly dislike any
code that splits my window without my explicit request.

(Consider this as a petition to, at the very least, add an option to
enable the interrupting, non-window-splitting behavior.)

             Juanma

Re: Pretest?

[Richard Stallman]

    (Consider this as a petition to, at the very least, add an option to
    enable the interrupting, non-window-splitting behavior.)

I don't mind having it as an option.

Re: Pretest?

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     > It should display that buffer in another window, and not abort
>     > anything.
>
>     That might means the new buffer will not be immediately available for
>     editing, until the user cancels the minibuffer or isearch.
>
> That is right.  The user will see the other buffer appear, and can
> either finish or cancel the command that is in progress.  I think that
> is less harsh.

By the way, do you actually use emacsclient/server yourself?  It seems
that people who actually do use emacsclient (well, and also everyone
else who has weighed in on this matter) feel it is more intuitive for
an emacsclient call to interrupt isearch, rather than the behavior you
are proposing.  In that sense, your "less harsh" behavior---whatever
other benefits it might have---is actually more harsh, because it
doesn't do what people expect.

Re: Pretest?

[Richard Stallman]

    By the way, do you actually use emacsclient/server yourself?

No, I don't use it.  I have only occasionally tested it.

I wonder about the statement that emacsclient is always invoked
at specific user request.  Indeed, that is usually so, but is it
always so?  As usage becomes more diverse, I expect it will cease
to be true.

Re: Pretest?

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     By the way, do you actually use emacsclient/server yourself?
>
> No, I don't use it.  I have only occasionally tested it.
>
> I wonder about the statement that emacsclient is always invoked at
> specific user request.  Indeed, that is usually so, but is it always
> so?  As usage becomes more diverse, I expect it will cease to be
> true.

Why?  Does your system tend to asynchronously pop up editors which
you, as the user, did not request by some particular interaction?

I never have experienced anything like that.

The only case where one might be having second thoughts is for the
_multi-tty_ branch: there it might be nice to not have the home
display changed when one has connected via emacsclient from work in
between.  However, this is completely unfeasible due to Emacs' lack of
multi-threading terminals: at some point of time, top-level will get
executed in the remote tty, anyway.

So I can't see where you get your ideas about typical usage of
emacsclient, and it does not particularly help that you state that you
don't actually use it, in contrast to the people reporting their
expected behaviors.

Perhaps you are thinking that one will use
emacsclient --eval
for doing some noninteractive system task in some asynchronous manner.
However, it would be an awful mistake to do this with an interactive
Emacs session since the state of such a session and the files it may
have open (including password-protected files in different accounts
accessed via tramp) are completely unknown.

In spite of its name, emacsclient is very much restricted to behaving
like an interactive _editor_.  Using it for noninteractive
non-user-initiated tasks would not be feasible.  If you really wanted
to build a system service executing Elisp based on it, you'd start an
Emacs of its own, non-interactively, detached from a tty.

We can't even do that in Emacs 22.  So please let us not base our
defaults on some weird case that is not likely to become the default
usage in future and can't even be usefully employed in that manner in
Emacs 22.

-- 
David Kastrup

Re: Pretest?

[Nick Roberts]

> > I think a new pretest March 1 would be good.

> OK, March 1 it is.

If the release really will be soon, how about forking at the same time and
imposing a formal approval procedure on the release branch to prevent
unnecessary, and possibly adverse, commits?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: Pretest?

[David Kastrup]

Nick Roberts <nickrob@snap.net.nz> writes:

>> > I think a new pretest March 1 would be good.
>
>> OK, March 1 it is.
>
> If the release really will be soon, how about forking at the same
> time and imposing a formal approval procedure on the release branch
> to prevent unnecessary, and possibly adverse, commits?

Once we fork, no developer will look back.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Pretest?

[Nick Roberts]

David Kastrup writes:
 > Nick Roberts <nickrob@snap.net.nz> writes:
 > 
 > >> > I think a new pretest March 1 would be good.
 > >
 > >> OK, March 1 it is.
 > >
 > > If the release really will be soon, how about forking at the same
 > > time and imposing a formal approval procedure on the release branch
 > > to prevent unnecessary, and possibly adverse, commits?
 > 
 > Once we fork, no developer will look back.

Well obviously I disagree as most developers want to see a release.  That
clearly won't happen if they stop working on it.  

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: Pretest?

[Richard Stallman]

    If the release really will be soon, how about forking at the same time and
    imposing a formal approval procedure on the release branch to prevent
    unnecessary, and possibly adverse, commits?

I would rather keep Emacs 22 in the trunk until we release it.