harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Salikh Zakirov <sal...@gmail.com>
Subject Re: [drlvm][threading] what are the design rules for calling vm_gc_loc_enum()?
Date Sun, 04 Mar 2007 16:13:02 GMT
Weldon Washburn wrote:
> After committing H3203 patch, I worry there may be problems with the
> modified version of vm_gc_lock_enum().
>


The function hythread_global_lock() was improved to reset suspend_disable_count
to 0 while it is blocking on the lock call:

+    saved_count = reset_suspend_disable();
+    r = hymutex_lock(TM_LIBRARY->TM_LOCK);
+    if (r) return r;

BTW, the above sequence has a minor bug in case when hymutex_lock() return
non-zero value, indicating an error. The correct code should do

if (r) {
    self->suspend_disable_count = saved_count;
    return r;
}

This issue is minor because failure of mutex_lock is very uncommon.
I've only seen it happen on shutdown race condition, when some other thread
already destroyed the mutex we are trying to lock on.

Looking further down the patch, there are other cases of 'return r' without
restoring suspend_disable_count. Probably the following construct could be used:

   if (r) goto cleanup;

   ...

cleanup:
   self->suspend_disabled_count = saved_count;
   return r;
}

> Basically tmn_suspend_enable() and ...disable() were removed.  Is it
> somehow
> implied that a given thread must be in suspend enabled mode before calling
> vm_gc_lock_enum()?  If this is true, does it make sense to put an "assert(
> HyThread.suspend_disable_count == 0);" at the top of vm_gc_lock_enum()?

No, it is not implied, see hythread_global_lock(). In fact, vm_gc_lock_enum()
is always called with suspend_disabled_count == 1, so if you want to put some
assertion, it should be

	assert(hythread_self()->suspend_disabled_count == 1);

--
Salikh Zakirov


Mime
View raw message