[PATCH] (almost) thread-safe libgfortran

[Richard Henderson]

On Thu, Oct 06, 2005 at 07:53:30PM -0400, Jakub Jelinek wrote:
> @@ -145,9 +135,10 @@ st_printf (const char *format, ...)
>    const char *q;
>    stream *s;
>    char itoa_buf[GFC_ITOA_BUF_SIZE];
> +  char err_stream[sizeof (unix_stream)];
...
> +  s = init_error_stream (err_stream);

Is this variable sufficiently aligned?  Why do you want to declare
a char array instead of just

	unix_stream err_stream;
	s = init_error_stream (&err_stream);

> +  if (!locked)
> +    __gthread_mutex_unlock (&u->lock);
> +
> +  /* If there are any threads waiting in find_unit for this unit,
> +     avoid freeing the memory, the last such thread will free it
> +     instead.  */
> +  if (u->waiting == 0)
> +    free_mem (u);
> +
> +  if (!locked)
> +    __gthread_mutex_unlock (&unit_lock);

Unlocking twice on one path?

> +  __gthread_mutex_lock (&unit_lock);
> +retry:
> +  u = find_file0 (unit_root, &statbuf);
> +  if (u != NULL)
> +    {
> +#ifdef HAVE_SYNC_FETCH_AND_ADD
> +      (void) __sync_fetch_and_add (&u->waiting, 1);
> +#else
> +      u->waiting++;
> +#endif

Wouldn't you want to do a try_lock first?  It could avoid two
atomic instructions, which would be nice.

> +	  if (__sync_fetch_and_add (&u->waiting, -1) == 1)

  __sync_add_and_fetch (&u->waiting, -1) == 0

And it seems like you really ought to consider encapsulating
these calls in inline functions, so that there's not so much
ifdeffing.  Something like

inline int predec_waiting_locked (u)
{
#ifdef HAVE_SYNC_FETCH_AND_ADD
  return __sync_add_and_fetch (&u->waiting, -1);
#else
  return --u->waiting;
#endif
}

inline void dec_waiting_unlocked (u)
{
#ifdef HAVE_SYNC_FETCH_AND_ADD
  __sync_fetch_and_add (&u->waiting, -1);
#else
  __gthread_mutex_lock (&unit_lock);
  u->waiting--;
  __gthread_mutex_unlock (&unit_lock);
#endif
}

etc.



r~