Re: Pretest?

["Jan Djärv" <jan.h.d@swipnet.se>]

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
>