emacs-22.1 with GTK problems (with patches)

emacs-22.1 with GTK problems (with patches)

[Peter O'Gorman]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

Hi,
We encountered a few minor issues building emacs-22.1 with GTK. On
Solaris 2.6 there is no recursive mutex support. I worried about using
pthread_getspecific/setspecific etc. to emulate recursive mutexes
because it seemed likely that these would require a malloc, and the
locks are around malloc hooks. Instead, since the only chance of
recursive behavior seemed to be realloc calling malloc/free we copied
the locking malloc and free functions, but removed the locks, and used
those for realloc on systems without recursive mutexes.

On HP-UX, emacs failed to build with GTK because hp-ux has, in its
pthread.h:
        static int pthread_create( .....
And we have #define static, so it died with multiple definitions of
pthread_create and pthread_attr_init. Removing the #define was not
enough, we got segmentation faults until we also removed the
-lpthread.

AIX-4.3.3 failed to build because abs() was #defined multiple times,
and a couple of enums ended with a comma.

Failed to build on linux with gcc-2.95 because gtkutil.c has a
variable declaration in the middle of a block.

Patch attached.

Thanks,
Peter


[-- Attachment #2: emacs-22.1.patch --]
[-- Type: text/plain, Size: 8807 bytes --]

Index: src/xterm.c
===================================================================
--- src/xterm.c.orig	2007-05-25 12:43:35.000000000 +0000
+++ src/xterm.c	2007-08-31 03:43:32.106444418 +0000
@@ -155,7 +155,9 @@
 #endif
 #endif
 
+#ifndef abs
 #define abs(x)	((x) < 0 ? -(x) : (x))
+#endif
 
 /* Default to using XIM if available.  */
 #ifdef USE_XIM
Index: src/keyboard.c
===================================================================
--- src/keyboard.c.orig	2007-04-01 21:54:54.000000000 +0000
+++ src/keyboard.c	2007-08-31 03:43:32.291971223 +0000
@@ -112,7 +112,9 @@
 #define KBD_BUFFER_SIZE 4096
 #endif	/* No X-windows */
 
+#ifndef abs
 #define abs(x)		((x) >= 0 ? (x) : -(x))
+#endif
 
 /* Following definition copied from eval.c */
 
Index: configure.in
===================================================================
--- configure.in.orig	2007-05-25 12:43:29.000000000 +0000
+++ configure.in	2007-09-07 05:02:42.527531369 +0000
@@ -2165,9 +2158,24 @@
       AC_CHECK_LIB(pthread, pthread_self, HAVE_GTK_AND_PTHREAD=yes)
     fi
     if test "$HAVE_GTK_AND_PTHREAD" = yes; then
-      GTK_LIBS="$GTK_LIBS -lpthread"
+      case "${canonical}" in
+		*-hpux*) ;;
+		*) GTK_LIBS="$GTK_LIBS -lpthread" ;;
+	  esac
       AC_DEFINE(HAVE_GTK_AND_PTHREAD, 1,
                [Define to 1 if you have GTK and pthread (-lpthread).])
+	  AC_CACHE_CHECK(for recursive mutexes,ac_cv_have_recursive_mutexes,
+	  AC_COMPILE_IFELSE(AC_LANG_PROGRAM([#include <pthread.h>],[
+  pthread_mutexattr_t attr;
+  pthread_mutexattr_init (&attr);
+  pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
+  return 0;
+]),
+[ac_cv_have_recursive_mutexes=yes],[ac_cv_have_recursive_mutexes=no]))
+      if test "x$ac_cv_have_recursive_mutexes" = "xyes"; then
+        AC_DEFINE(HAVE_RECURSIVE_MUTEXES,1,
+                  [Define if you have recursive mutexes])
+	  fi
     fi
   fi
 fi
Index: src/gtkutil.c
===================================================================
--- src/gtkutil.c.orig	2007-05-14 14:56:31.000000000 +0000
+++ src/gtkutil.c	2007-09-03 20:58:27.261561563 +0000
@@ -1403,8 +1403,8 @@
     {
       Lisp_Object file;
       struct gcpro gcpro1;
-      GCPRO1 (file);
       char *utf8_filename;
+      GCPRO1 (file);
 
       file = build_string (default_filename);
 
Index: src/s/hpux.h
===================================================================
--- src/s/hpux.h.orig	2007-01-14 03:38:22.000000000 +0000
+++ src/s/hpux.h	2007-09-06 03:04:37.691410052 +0000
@@ -169,7 +169,7 @@
    into the initialized data area, which becomes pure after dumping Emacs.
    Foil this.  Emacs carefully avoids static vars inside functions.  */
 
-#define static
+/* #define static */
 
 /* Define extra libraries to load.
    This should have -lBSD, but that library is said to make
Index: src/frame.h
===================================================================
--- src/frame.h.orig	2007-01-21 04:18:16.000000000 +0000
+++ src/frame.h	2007-09-06 18:18:37.784302154 +0000
@@ -1036,7 +1036,7 @@
   FULLSCREEN_WIDTH      = 1,
   FULLSCREEN_HEIGHT     = 2,
   FULLSCREEN_BOTH       = 3,
-  FULLSCREEN_WAIT       = 4,
+  FULLSCREEN_WAIT       = 4
 };
 
 
Index: src/termhooks.h
===================================================================
--- src/termhooks.h.orig	2007-01-14 03:24:37.000000000 +0000
+++ src/termhooks.h	2007-09-06 18:17:17.922589738 +0000
@@ -331,7 +331,7 @@
 
   /* Queued from XTread_socket when session manager sends
      save yourself before shutdown. */
-  SAVE_SESSION_EVENT,
+  SAVE_SESSION_EVENT
 
 #ifdef MAC_OS
   /* Generated when an Apple event, a HICommand event, or a Services
@@ -340,7 +340,7 @@
      symbols, respectively.  Member `arg' is a Lisp object converted
      from the received Apple event.  Parameters for non-Apple events
      are converted to those in Apple events.  */
-  MAC_APPLE_EVENT
+  ,MAC_APPLE_EVENT
 #endif
 };
 
Index: src/xfaces.c
===================================================================
--- src/xfaces.c.orig	2007-02-14 15:47:51.000000000 +0000
+++ src/xfaces.c	2007-09-06 18:17:56.728517424 +0000
@@ -263,9 +263,9 @@
 #endif /* HAVE_X_WINDOWS */
 
 #include <ctype.h>
-
+#ifndef abs
 #define abs(X)		((X) < 0 ? -(X) : (X))
-
+#endif
 /* Number of pt per inch (from the TeXbook).  */
 
 #define PT_PER_INCH 72.27
Index: src/alloc.c
===================================================================
--- src/alloc.c.orig	2007-09-06 17:37:51.000000000 +0000
+++ src/alloc.c	2007-09-06 22:57:03.580284260 +0000
@@ -1171,7 +1171,6 @@
 static void * (*old_malloc_hook) P_ ((size_t, const void *));
 static void * (*old_realloc_hook) P_ ((void *,  size_t, const void*));
 static void (*old_free_hook) P_ ((void*, const void*));
-
 /* This function is used as the hook for free to call.  */
 
 static void
@@ -1271,6 +1270,104 @@
   return value;
 }
 
+#ifndef HAVE_RECURSIVE_MUTEXES
+/* This function is used as the hook for free to call from realloc.  */
+
+static void
+emacs_blocked_reallocfree (ptr, ptr2)
+     void *ptr;
+     const void *ptr2;
+{
+  EMACS_INT bytes_used_now;
+
+
+#ifdef GC_MALLOC_CHECK
+  if (ptr)
+    {
+      struct mem_node *m;
+
+      m = mem_find (ptr);
+      if (m == MEM_NIL || m->start != ptr)
+	{
+	  fprintf (stderr,
+		   "Freeing `%p' which wasn't allocated with malloc\n", ptr);
+	  abort ();
+	}
+      else
+	{
+	  /* fprintf (stderr, "free %p...%p (%p)\n", m->start, m->end, ptr); */
+	  mem_delete (m);
+	}
+    }
+#endif /* GC_MALLOC_CHECK */
+
+  __free_hook = old_free_hook;
+  free (ptr);
+
+  /* If we released our reserve (due to running out of memory),
+     and we have a fair amount free once again,
+     try to set aside another reserve in case we run out once more.  */
+  if (! NILP (Vmemory_full)
+      /* Verify there is enough space that even with the malloc
+	 hysteresis this call won't run out again.
+	 The code here is correct as long as SPARE_MEMORY
+	 is substantially larger than the block size malloc uses.  */
+      && (bytes_used_when_full
+	  > ((bytes_used_when_reconsidered = BYTES_USED)
+	     + max (malloc_hysteresis, 4) * SPARE_MEMORY)))
+    refill_memory_reserve ();
+
+  __free_hook = emacs_blocked_free;
+}
+
+
+/* This function is the malloc hook that Emacs uses from realloc.  */
+
+static void *
+emacs_blocked_reallocmalloc (size, ptr)
+     size_t size;
+     const void *ptr;
+{
+  void *value;
+
+  __malloc_hook = old_malloc_hook;
+#ifdef DOUG_LEA_MALLOC
+    mallopt (M_TOP_PAD, malloc_hysteresis * 4096);
+#else
+    __malloc_extra_blocks = malloc_hysteresis;
+#endif
+
+  value = (void *) malloc (size);
+
+#ifdef GC_MALLOC_CHECK
+  {
+    struct mem_node *m = mem_find (value);
+    if (m != MEM_NIL)
+      {
+	fprintf (stderr, "Malloc returned %p which is already in use\n",
+		 value);
+	fprintf (stderr, "Region in use is %p...%p, %u bytes, type %d\n",
+		 m->start, m->end, (char *) m->end - (char *) m->start,
+		 m->type);
+	abort ();
+      }
+
+    if (!dont_register_blocks)
+      {
+	mem_insert (value, (char *) value + max (1, size), allocated_mem_type);
+	allocated_mem_type = MEM_TYPE_NON_LISP;
+      }
+  }
+#endif /* GC_MALLOC_CHECK */
+
+  __malloc_hook = emacs_blocked_malloc;
+
+  /* fprintf (stderr, "%p malloc\n", value); */
+  return value;
+}
+
+
+#endif
 
 /* This function is the realloc hook that Emacs uses.  */
 
@@ -1284,7 +1381,10 @@
 
   BLOCK_INPUT_ALLOC;
   __realloc_hook = old_realloc_hook;
-
+#ifndef HAVE_RECURSIVE_MUTEXES
+  __malloc_hook = emacs_blocked_reallocmalloc;
+  __free_hook = emacs_blocked_reallocfree;
+#endif
 #ifdef GC_MALLOC_CHECK
   if (ptr)
     {
@@ -1298,6 +1398,10 @@
 	}
 
       mem_delete (m);
+#ifndef HAVE_RECURSIVE_MUTEXES
+      __malloc_hook = emacs_blocked_reallocmalloc;
+      __free_hook = emacs_blocked_reallocfree;
+#endif
     }
 
   /* fprintf (stderr, "%p -> realloc\n", ptr); */
@@ -1307,7 +1411,10 @@
 #endif /* GC_MALLOC_CHECK */
 
   value = (void *) realloc (ptr, size);
-
+#ifndef HAVE_RECURSIVE_MUTEXES
+  __malloc_hook = emacs_blocked_reallocmalloc;
+  __free_hook = emacs_blocked_reallocfree;
+#endif
 #ifdef GC_MALLOC_CHECK
   dont_register_blocks = 0;
 
@@ -1326,6 +1433,10 @@
   /* fprintf (stderr, "%p <- realloc\n", value); */
 #endif /* GC_MALLOC_CHECK */
 
+#ifndef HAVE_RECURSIVE_MUTEXES
+  __malloc_hook = emacs_blocked_malloc;
+  __free_hook = emacs_blocked_free;
+#endif
   __realloc_hook = emacs_blocked_realloc;
   UNBLOCK_INPUT_ALLOC;
 
@@ -1360,7 +1471,9 @@
   /*  GLIBC has a faster way to do this, but lets keep it portable.
       This is according to the Single UNIX Specification.  */
   pthread_mutexattr_init (&attr);
+#ifdef  HAVE_RECURSIVE_MUTEXES 
   pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
+#endif
   pthread_mutex_init (&alloc_mutex, &attr);
 #endif /* HAVE_GTK_AND_PTHREAD */
 

Re: emacs-22.1 with GTK problems (with patches)

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 10 Sep 2007 11:58:26 -0500, "Peter O'Gorman" <bug-gnu-emacs@mlists.thewrittenword.com> said:

> We encountered a few minor issues building emacs-22.1 with GTK. On
> Solaris 2.6 there is no recursive mutex support.

Several bugfixes have been made to gmalloc.c since the Emacs 22.1
release.  Could you try the latest gmalloc.c and emacs.c below?  I
think Solaris 2.6 is untested yet.

http://cvs.savannah.gnu.org/viewvc/*checkout*/emacs/src/gmalloc.c?revision=1.21.2.5&root=emacs&pathrev=EMACS_22_BASE
http://cvs.savannah.gnu.org/viewvc/*checkout*/emacs/src/emacs.c?revision=1.401.2.2&root=emacs&pathrev=EMACS_22_BASE

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

Re: emacs-22.1 with GTK problems (with patches)

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 11 Sep 2007 09:57:57 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

>> We encountered a few minor issues building emacs-22.1 with GTK. On
>> Solaris 2.6 there is no recursive mutex support.

> Several bugfixes have been made to gmalloc.c since the Emacs 22.1
> release.  Could you try the latest gmalloc.c and emacs.c below?  I
> think Solaris 2.6 is untested yet.

Oops, your patch was for another use of recursive mutex in alloc.c and
had nothing to do with the mutexes in gmalloc.c.  Sorry about that.

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

Re: emacs-22.1 with GTK problems (with patches)

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 11 Sep 2007 12:42:26 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

>>> We encountered a few minor issues building emacs-22.1 with GTK. On
>>> Solaris 2.6 there is no recursive mutex support.

>> Several bugfixes have been made to gmalloc.c since the Emacs 22.1
>> release.  Could you try the latest gmalloc.c and emacs.c below?  I
>> think Solaris 2.6 is untested yet.

> Oops, your patch was for another use of recursive mutex in alloc.c
> and had nothing to do with the mutexes in gmalloc.c.  Sorry about
> that.

On second thought, it seems that the mutex in alloc.c don't have to be
recursive regardless of your patch, because _realloc_internal only
calls "hookless" versions of malloc/free (i.e., _malloc_internal and
_free_internal).  Could you try just replacing

  pthread_mutexattr_t attr;

  /*  GLIBC has a faster way to do this, but lets keep it portable.
      This is according to the Single UNIX Specification.  */
  pthread_mutexattr_init (&attr);
  pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
  pthread_mutex_init (&alloc_mutex, &attr);

in uninterrupt_malloc with the following one?

  pthread_mutex_init (&alloc_mutex, NULL);

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

Re: emacs-22.1 with GTK problems (with patches)

[Jan Djärv]

 > On Solaris 2.6 there is no recursive mutex support.

Then we can not support that platform.  I can put in a configure test for 
recursive mutexes, but we have to fail with an error if there is no such thing.

I will fix the other things.

	Jan D.

Re: emacs-22.1 with GTK problems (with patches)

[YAMAMOTO Mitsuharu]

>>>>> On Fri, 14 Sep 2007 08:06:19 +0200, Jan Djärv <jan.h.d@swipnet.se> said:

>> On Solaris 2.6 there is no recursive mutex support.
> Then we can not support that platform.  I can put in a configure
> test for recursive mutexes, but we have to fail with an error if
> there is no such thing.

What do you think about the change below?

(I asked the OP whether a similar change works in
http://lists.gnu.org/archive/html/bug-gnu-emacs/2007-09/msg00060.html,
but no reply yet. )

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

Index: src/alloc.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/alloc.c,v
retrieving revision 1.415
diff -c -p -r1.415 alloc.c
*** src/alloc.c	29 Aug 2007 05:27:51 -0000	1.415
--- src/alloc.c	14 Sep 2007 06:09:50 -0000
*************** void
*** 1356,1361 ****
--- 1356,1362 ----
  uninterrupt_malloc ()
  {
  #ifdef HAVE_GTK_AND_PTHREAD
+ #ifdef DOUG_LEA_MALLOC
    pthread_mutexattr_t attr;
  
    /*  GLIBC has a faster way to do this, but lets keep it portable.
*************** uninterrupt_malloc ()
*** 1363,1368 ****
--- 1364,1374 ----
    pthread_mutexattr_init (&attr);
    pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
    pthread_mutex_init (&alloc_mutex, &attr);
+ #else  /* !DOUG_LEA_MALLOC */
+   /* Some systems such as Solaris 2.6 doesn't have a recursive mutex,
+      and the bundled gmalloc.c doesn't require it.  */
+   pthread_mutex_init (&alloc_mutex, NULL);
+ #endif /* !DOUG_LEA_MALLOC */
  #endif /* HAVE_GTK_AND_PTHREAD */
  
    if (__free_hook != emacs_blocked_free)

Re: emacs-22.1 with GTK problems (with patches)

[Jan Djärv]

YAMAMOTO Mitsuharu skrev:
>>>>>> On Fri, 14 Sep 2007 08:06:19 +0200, Jan Djärv <jan.h.d@swipnet.se> said:
> 
>>> On Solaris 2.6 there is no recursive mutex support.
>> Then we can not support that platform.  I can put in a configure
>> test for recursive mutexes, but we have to fail with an error if
>> there is no such thing.
> 
> What do you think about the change below?

I guess it is OK, but I'd like the OP to test it first, you never know with 
Solars :-).  2.6 is way too old for me, the oldest I have is Solaris 8 (a.k.a. 
2.8).

On second thought, we could also don't define HAVE_GTK_AND_PTHREAD in this case.

	Jan D.

> 
> (I asked the OP whether a similar change works in
> http://lists.gnu.org/archive/html/bug-gnu-emacs/2007-09/msg00060.html,
> but no reply yet. )
> 
> 			     YAMAMOTO Mitsuharu
> 			mituharu@math.s.chiba-u.ac.jp
> 
> Index: src/alloc.c
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/src/alloc.c,v
> retrieving revision 1.415
> diff -c -p -r1.415 alloc.c
> *** src/alloc.c	29 Aug 2007 05:27:51 -0000	1.415
> --- src/alloc.c	14 Sep 2007 06:09:50 -0000
> *************** void
> *** 1356,1361 ****
> --- 1356,1362 ----
>   uninterrupt_malloc ()
>   {
>   #ifdef HAVE_GTK_AND_PTHREAD
> + #ifdef DOUG_LEA_MALLOC
>     pthread_mutexattr_t attr;
>   
>     /*  GLIBC has a faster way to do this, but lets keep it portable.
> *************** uninterrupt_malloc ()
> *** 1363,1368 ****
> --- 1364,1374 ----
>     pthread_mutexattr_init (&attr);
>     pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_RECURSIVE);
>     pthread_mutex_init (&alloc_mutex, &attr);
> + #else  /* !DOUG_LEA_MALLOC */
> +   /* Some systems such as Solaris 2.6 doesn't have a recursive mutex,
> +      and the bundled gmalloc.c doesn't require it.  */
> +   pthread_mutex_init (&alloc_mutex, NULL);
> + #endif /* !DOUG_LEA_MALLOC */
>   #endif /* HAVE_GTK_AND_PTHREAD */
>   
>     if (__free_hook != emacs_blocked_free)
> 

Re: emacs-22.1 with GTK problems (with patches)

[Glenn Morris]

Jan Djärv wrote:

>>>> On Solaris 2.6 there is no recursive mutex support.
>>> Then we can not support that platform.  I can put in a configure
>>> test for recursive mutexes, but we have to fail with an error if
>>> there is no such thing.
>> What do you think about the change below?
>
> I guess it is OK, but I'd like the OP to test it first, you never
> know with Solars :-). 2.6 is way too old for me, the oldest I have
> is Solaris 8 (a.k.a. 2.8).
>
> On second thought, we could also don't define HAVE_GTK_AND_PTHREAD
> in this case.


Hi,

Was a decision reached about what to do here? It would be nice to
cross this off from FOR-RELEASE...

Re: emacs-22.1 with GTK problems (with patches)

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 27 Sep 2007 23:55:26 -0400, Glenn Morris <rgm@gnu.org> said:

> Jan Djärv wrote:
>>>>> On Solaris 2.6 there is no recursive mutex support.
>>>> Then we can not support that platform.  I can put in a configure
>>>> test for recursive mutexes, but we have to fail with an error if
>>>> there is no such thing.
>>> What do you think about the change below?
>> 
>> I guess it is OK, but I'd like the OP to test it first, you never
>> know with Solars :-). 2.6 is way too old for me, the oldest I have
>> is Solaris 8 (a.k.a. 2.8).
>> 
>> On second thought, we could also don't define HAVE_GTK_AND_PTHREAD
>> in this case.

> Hi,

> Was a decision reached about what to do here? It would be nice to
> cross this off from FOR-RELEASE...

I think we need more info from the OP:

  * Whether my patch worked or not.
  * Whether GTK+ on Solaris 2.6 uses pthread or not.

BTW, the OP's address, bug-gnu-emacs@mlists.thewrittenword.com,
doesn't look like an address assigned to a person, and it is not
certain whether the OP can be reached with this address as he didn't
post any followups.

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

Re: emacs-22.1 with GTK problems (with patches)

[Glenn Morris]

Hi,


In September you posted some issues to the Emacs bug mailing list:

http://lists.gnu.org/archive/html/bug-gnu-emacs/2007-09/msg00055.html


We would appreciate it if you could reply about whether the fixes that
were suggested work for you. In particular, the issue with recursive
mutexes.


Thanks.