[PATCH] Possible threads race condition in src/alarm.c
Reini Urban
rurban at x-ray.at
Thu Aug 30 17:33:34 UTC 2012
On Thu, Aug 30, 2012 at 5:34 PM, Reini Urban <rurban at x-ray.at> wrote:
> On Thu, Aug 30, 2012 at 1:00 AM, Andrew Whitworth <wknight8111 at gmail.com> wrote:
>> On Wed, Aug 29, 2012 at 4:50 PM, Reini Urban <rurban at x-ray.at> wrote:
>>> I verified that it fixes the sleep deadlock, but with
>>> the deadlock fix there come 3 more races detected by tsan.
>>> (See http://blogs.perl.org/users/rurban/2012/08/threadsanitizer.html)
> tsan found two remaining races, one might be be fixed after the merge,
> one can be ignored IMHO.
>
> case 1:
> Parrot_Task_nci_send writes to the task mailbox (src/pmc/task.c:315),
> while Parrot_Task_invoke (src/pmc/task.c:174) might read from it.
> I started writing a mutex lock around the mailbox like this, but it
> did not help so far. I'll have another look.
Fixed with 2206f235621c03bf553101309002120f3d412edb
> diff --git a/src/pmc/task.pmc b/src/pmc/task.pmc
> index f3e151c..73127a2 100644
> --- a/src/pmc/task.pmc
> +++ b/src/pmc/task.pmc
> @@ -442,7 +442,9 @@ Send a message to this task.
> METHOD send(PMC *message) {
> Parrot_Task_attributes * const tdata = PARROT_TASK(SELF);
> if (PMC_IS_NULL(tdata->mailbox)) {
> + LOCK(tdata->mailbox_lock);
> tdata->mailbox = Parrot_pmc_new(interp, enum_class_PMCList);
> + UNLOCK(tdata->mailbox_lock);
> PARROT_GC_WRITE_BARRIER(interp, SELF);
> }
>
> I haven't seen that yet in the wild, but it could happen.
>
> case 2:
> When creating new threads, threads_array will be filled with a new
> non-null entry, but handling pending signals in
> Parrot_thread_notify_threads might miss the new thread entry
> in threads_array if the access is concurrent.
Also fixed with 2206f235621c03bf553101309002120f3d412edb
> I don't think that this scenario is worth fixing.
> And it is also very hard to fix, since interp is not passed through
> Parrot_thread_notify_threads. The interp arg should be changed to SHIM_INTERP
> also.
There are some more possible races, found by tsan.
tsan ./parrot t/src/threads.t (see attachment)
1. alarm_serial written by Parrot_alarm_runloop alarm.c:127
and read by Parrot_alarm_check alarm.c:152
2. Integer read/write within Parrot_Task_invoke
==9502== WARNING: Possible data race during read of size 8 at 0x428EBF0: {{{
==9502== T2 (L{}):
==9502== #0 Parrot_Integer_multi_cmp_DEFAULT
/usr/src/parrot/threads/src/pmc/integer.c:1361
==9502== #1 Parrot_Integer_cmp
/usr/src/parrot/threads/src/pmc/integer.c:382
==9502== #2 Parrot_Proxy_cmp /usr/src/parrot/threads/src/pmc/proxy.c:617
==9502== #3 Parrot_gt_p_ic_ic
/usr/src/parrot/threads/src/ops/core_ops.c:16091
==9502== #4 runops_fast_core
/usr/src/parrot/threads/src/runcore/cores.c:499
==9502== #5 runops_int /usr/src/parrot/threads/src/runcore/main.c:220
==9502== #6 runops /usr/src/parrot/threads/src/call/ops.c:123
==9502== #7 Parrot_pcc_invoke_from_sig_object
/usr/src/parrot/threads/src/call/pcc.c:338
==9502== #8 Parrot_ext_call /usr/src/parrot/threads/src/extend.c:158
==9502== #9 Parrot_Task_invoke /usr/src/parrot/threads/src/pmc/task.c:168
==9502== #10 Parrot_pcc_invoke_from_sig_object
/usr/src/parrot/threads/src/call/pcc.c:330
==9502== #11 Parrot_ext_call /usr/src/parrot/threads/src/extend.c:158
==9502== Concurrent write(s) happened at (OR AFTER) these points:
==9502== T0 (L{}):
==9502== #0 Parrot_Integer_set_integer_native_orig
/usr/src/parrot/threads/src/pmc/integer.c:1878
==9502== #1 Parrot_Integer_set_integer_native
/usr/src/parrot/threads/src/pmc/integer.c:841
==9502== #2 Parrot_set_p_ic
/usr/src/parrot/threads/src/ops/core_ops.c:20289
==9502== #3 runops_fast_core
/usr/src/parrot/threads/src/runcore/cores.c:499
==9502== #4 runops_int /usr/src/parrot/threads/src/runcore/main.c:220
==9502== #5 runops /usr/src/parrot/threads/src/call/ops.c:123
==9502== #6 Parrot_pcc_invoke_from_sig_object
/usr/src/parrot/threads/src/call/pcc.c:338
==9502== #7 Parrot_ext_call /usr/src/parrot/threads/src/extend.c:158
==9502== #8 Parrot_Task_invoke /usr/src/parrot/threads/src/pmc/task.c:168
==9502== #9 Parrot_pcc_invoke_from_sig_object
/usr/src/parrot/threads/src/call/pcc.c:330
3. interp->scheduler
==9502== WARNING: Possible data race during read of size 8 at 0x42F89D0: {{{
==9502== T3 (L{}):
==9502== #0 Parrot_Scheduler_unshift_pmc
/usr/src/parrot/threads/src/pmc/scheduler.c:168
==9502== #1 Parrot_cx_schedule_immediate
/usr/src/parrot/threads/src/scheduler.c:517
==9502== #2 Parrot_ParrotInterpreter_nci_schedule_proxied
/usr/src/parrot/threads/src/pmc/parrotinterpreter.c:919
==9502== Concurrent write(s) happened at (OR AFTER) these points:
==9502== T2 (L{}):
==9502== #0 Parrot_cx_schedule_immediate
/usr/src/parrot/threads/src/scheduler.c:518
==9502== #1 Parrot_ParrotInterpreter_nci_schedule_proxied
/usr/src/parrot/threads/src/pmc/parrotinterpreter.c:919
4. general pmc memory race in Parrot_thread_create_local_task
==9502== WARNING: Possible data race during read of size 8 at 0x42233F8: {{{
==9502== T4 (L{}):
==9502== #0 gc_gms_allocate_pmc_header
/usr/src/parrot/threads/src/gc/gc_gms.c:1490
==9502== #1 Parrot_gc_new_pmc_header
/usr/src/parrot/threads/src/gc/api.c:312
==9502== #2 get_new_pmc_header /usr/src/parrot/threads/src/pmc.c:571
==9502== #3 Parrot_pmc_new /usr/src/parrot/threads/src/pmc.c:217
==9502== #4 Parrot_thread_create_local_task
/usr/src/parrot/threads/src/thread.c:220
==9502== #5 Parrot_ParrotInterpreter_nci_schedule_proxied
/usr/src/parrot/threads/src/pmc/parrotinterpreter.c:919
==9502== #6 Parrot_NativePCCMethod_invoke
/usr/src/parrot/threads/src/pmc/nativepccmethod.c:124
==9502== #7 Parrot_callmethodcc_p_sc
/usr/src/parrot/threads/src/ops/core_ops.c:18298
==9502== #8 runops_fast_core
/usr/src/parrot/threads/src/runcore/cores.c:499
==9502== #9 runops_int /usr/src/parrot/threads/src/runcore/main.c:220
==9502== #10 runops /usr/src/parrot/threads/src/call/ops.c:123
==9502== #11 Parrot_pcc_invoke_from_sig_object
/usr/src/parrot/threads/src/call/pcc.c:338
==9502== Concurrent write(s) happened at (OR AFTER) these points:
==9502== T0 (L{}):
==9502== #0 gc_gms_allocate_string_storage
/usr/src/parrot/threads/src/gc/gc_gms.c:1753
==9502== #1 Parrot_gc_allocate_string_storage
/usr/src/parrot/threads/src/gc/api.c:509
==9502== #2 Parrot_str_new_init
/usr/src/parrot/threads/src/string/api.c:682
==9502== #3 Parrot_str_from_uint
/usr/src/parrot/threads/src/string/api.c:3253
==9502== #4 Parrot_str_from_int_base
/usr/src/parrot/threads/src/string/api.c:3284
==9502== #5 Parrot_str_from_int
/usr/src/parrot/threads/src/string/api.c:2157
==9502== #6 Parrot_Integer_get_string
/usr/src/parrot/threads/src/pmc/integer.c:552
==9502== #7 Parrot_hash_key_from_pmc
/usr/src/parrot/threads/src/hash.c:1796
==9502== #8 Parrot_Hash_delete_keyed_orig
/usr/src/parrot/threads/src/pmc/hash.c:987
==9502== #9 Parrot_Hash_delete_keyed
/usr/src/parrot/threads/src/pmc/hash.c:240
But no tests fail, so I wonder how relevant they are in reality.
--
Reini Urban
http://cpanel.net/ http://www.perl-compiler.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: thr.log
Type: application/octet-stream
Size: 11722 bytes
Desc: not available
URL: <http://lists.parrot.org/pipermail/parrot-dev/attachments/20120830/f24d82d9/attachment-0001.obj>
More information about the parrot-dev
mailing list