Re: plea for review



On Wed, 24 Jan 2001, Alexander Larsson wrote:

> On Wed, 24 Jan 2001, Tim Janik wrote:

> I've given it a read now, and i couldn't find much problems. I'm extremely
> uncomfortable with dropping the lock and calling external code, but I
> suppose if finalizers and initializer aren't completely insane it will
> work.

yeah, but there's not much we could actually do about it...

> check_derivation_U:
>   pnode = lookup_type_node_L (parent_type);
>   finfo = type_node_fundamental_info_L (pnode);
>   if (!pnode)
>     ...
> might call type_node_fundamental_info_L (NULL), which looks bad.

it acquires a lock before that, in short:
static gboolean
check_derivation_U (GType        parent_type,
                    const gchar *type_name)
{
  G_READ_LOCK (&type_rw_lock);
  pnode = lookup_type_node_L (parent_type);
  finfo = type_node_fundamental_info_L (pnode);
  [...]
  G_READ_UNLOCK (&type_rw_lock);
  return TRUE;
}

so it calls _L function while a lock is being held.

> Calls like g_type_fundamental_branch_last() are fundamentally
> non-threadsafe. The value returned may have changed before the caller even
> gets it.

that is right, but g_type_fundamental_branch_last() is basically only needed
if someone wants to introduce a new fundamental type, that's something
that in general should be done before numerous threads are spawned, and
even if that's not the case, a program needing new fundamentals in other
threads is quite broken ;)

> Otherwise I found nothing, although I can't claim I understand all the
> code.

ok, thanks very much for looking it over.

> 
> / Alex
> 

---
ciaoTJ





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]