[m-rev.] for review: work around thread procedures crashing in debug grade
Peter Wang
novalazy at gmail.com
Thu Oct 18 12:14:38 AEST 2007
I think a proper fix would be to write these troublesome procedures as
:- external procedures so they never get transformed, right?
Estimated hours taken: 1
Branches: main
The `thread.yield', `semaphore.wait', `semaphore.signal' procedures were
crashing in debug and deep profiling grades. The problem is that they caused
the calling context to suspend but resume in an auxiliary function instead of
back into the foreign_proc, so code inserted at the end of the foreign_proc by
those transforms wouldn't be executed.
Work around this by reverting to the original method of taking the address of
labels so that suspended contexts will resume back within the foreign_proc.
library/thread.m:
library/thread.semaphore.m:
#define ML_THREAD_AVOID_LABEL_ADDRS if execution tracing or deep
profiling are enabled in low-level C grades.
In the problematic problematic foreign_procs, test for
ML_THREAD_AVOID_LABEL_ADDRS and use the workaround if necessary.
Unrelated: add missing `tabled_for_io' attributes on foreign_procs in
these modules.
tests/hard_coded/Mmakefile:
tests/hard_coded/test_semaphore.exp:
tests/hard_coded/test_semaphore.m:
tests/hard_coded/test_yield.exp:
tests/hard_coded/test_yield.m:
Add test cases.
Index: library/thread.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/library/thread.m,v
retrieving revision 1.12
diff -u -r1.12 thread.m
--- library/thread.m 12 Sep 2007 06:21:13 -0000 1.12
+++ library/thread.m 18 Oct 2007 02:05:06 -0000
@@ -56,6 +56,28 @@
:- implementation.
+:- pragma foreign_decl("C", "
+#ifndef MR_HIGHLEVEL_CODE
+ #if !defined(MR_EXEC_TRACE) && !defined(MR_DEEP_PROFILING)
+ /*
+ ** In calling thread.yield, semaphore.wait or semaphore.signal, the
+ ** calling context may need to suspend and yield to another context.
+ ** This is implemented by setting the resume address of the context to an
+ ** auxiliary function outside of the foreign_proc. This breaks when
+ ** execution tracing or deep profiling are enabled as code inserted at the
+ ** end of the foreign_proc won't be executed. In those cases we rely on
+ ** the gcc extension that allows us to take the address of labels within
+ ** the foreign_proc, so the context will resume back inside the
+ ** foreign_proc.
+ **
+ ** XXX implement those procedures as :- external procedures so that the
+ ** transforms won't be applied
+ */
+ #define ML_THREAD_AVOID_LABEL_ADDRS
+ #endif
+#endif
+").
+
%-----------------------------------------------------------------------------%
:- pragma foreign_proc("C",
@@ -77,7 +99,7 @@
:- pragma foreign_proc("C",
spawn(Goal::(pred(di, uo) is cc_multi), IO0::di, IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
#if !defined(MR_HIGHLEVEL_CODE)
MR_Context *ctxt;
@@ -113,7 +135,7 @@
:- pragma foreign_proc("C#",
spawn(Goal::(pred(di, uo) is cc_multi), _IO0::di, _IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
System.Threading.Thread t;
MercuryThread mt = new MercuryThread(Goal);
@@ -126,15 +148,24 @@
:- pragma no_inline(yield/2).
:- pragma foreign_proc("C",
yield(IO0::di, IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
#ifndef MR_HIGHLEVEL_CODE
MR_save_context(MR_ENGINE(MR_eng_this_context));
+ #ifdef ML_THREAD_AVOID_LABEL_ADDRS
MR_ENGINE(MR_eng_this_context)->MR_ctxt_resume =
MR_ENTRY(mercury__thread__yield_resume);
+ #else
+ MR_ENGINE(MR_eng_this_context)->MR_ctxt_resume =
+ &&yield_skip_to_the_end;
+ #endif
MR_schedule_context(MR_ENGINE(MR_eng_this_context));
MR_ENGINE(MR_eng_this_context) = NULL;
MR_runnext();
+
+ #ifndef ML_THREAD_AVOID_LABEL_ADDRS
+ yield_skip_to_the_end:
+ #endif
#endif
IO = IO0;
").
Index: library/thread.semaphore.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/library/thread.semaphore.m,v
retrieving revision 1.13
diff -u -r1.13 thread.semaphore.m
--- library/thread.semaphore.m 17 Oct 2007 04:40:34 -0000 1.13
+++ library/thread.semaphore.m 18 Oct 2007 02:04:48 -0000
@@ -174,7 +174,7 @@
:- pragma no_inline(semaphore.signal/3).
:- pragma foreign_proc("C",
signal(Semaphore::in, IO0::di, IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
ML_Semaphore *sem;
#ifndef MR_HIGHLEVEL_CODE
@@ -199,24 +199,42 @@
/* yield() */
MR_save_context(MR_ENGINE(MR_eng_this_context));
+ #ifdef ML_THREAD_AVOID_LABEL_ADDRS
MR_ENGINE(MR_eng_this_context)->MR_ctxt_resume =
MR_ENTRY(mercury__thread__semaphore__nop);
+ #else
+ MR_ENGINE(MR_eng_this_context)->MR_ctxt_resume =
+ &&signal_skip_to_the_end_1;
+ #endif
MR_schedule_context(MR_ENGINE(MR_eng_this_context));
MR_ENGINE(MR_eng_this_context) = NULL;
MR_runnext();
+
+ #ifndef ML_THREAD_AVOID_LABEL_ADDRS
+ signal_skip_to_the_end_1: ;
+ #endif
} else {
sem->count++;
MR_UNLOCK(&(sem->lock), ""semaphore__signal"");
/* yield() */
MR_save_context(MR_ENGINE(MR_eng_this_context));
+ #ifdef ML_THREAD_AVOID_LABEL_ADDRS
MR_ENGINE(MR_eng_this_context)->MR_ctxt_resume =
MR_ENTRY(mercury__thread__semaphore__nop);
+ #else
+ MR_ENGINE(MR_eng_this_context)->MR_ctxt_resume =
+ &&signal_skip_to_the_end_2;
+ #endif
MR_schedule_context(MR_ENGINE(MR_eng_this_context));
MR_ENGINE(MR_eng_this_context) = NULL;
MR_runnext();
+
+ #ifndef ML_THREAD_AVOID_LABEL_ADDRS
+ signal_skip_to_the_end_2: ;
+ #endif
}
#else
sem->count++;
@@ -228,7 +246,7 @@
:- pragma foreign_proc("C#",
signal(Semaphore::in, _IO0::di, _IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
System.Threading.Monitor.Enter(Semaphore);
Semaphore.count++;
@@ -246,7 +264,7 @@
:- pragma no_inline(semaphore.wait/3).
:- pragma foreign_proc("C",
wait(Semaphore::in, IO0::di, IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
ML_Semaphore *sem;
#ifndef MR_HIGHLEVEL_CODE
@@ -266,7 +284,11 @@
/* Put the current context at the end of the queue. */
ctxt = MR_ENGINE(MR_eng_this_context);
+ #ifdef ML_THREAD_AVOID_LABEL_ADDRS
ctxt->MR_ctxt_resume = MR_ENTRY(mercury__thread__semaphore__nop);
+ #else
+ ctxt->MR_ctxt_resume = &&wait_skip_to_the_end;
+ #endif
ctxt->MR_ctxt_next = NULL;
if (sem->suspended_tail) {
sem->suspended_tail->MR_ctxt_next = ctxt;
@@ -280,6 +302,10 @@
/* Make the current engine do something else. */
MR_ENGINE(MR_eng_this_context) = NULL;
MR_runnext();
+
+ #ifndef ML_THREAD_AVOID_LABEL_ADDRS
+ wait_skip_to_the_end: ;
+ #endif
}
#else
while (sem->count <= 0) {
@@ -301,7 +327,7 @@
:- pragma foreign_proc("C#",
wait(Semaphore::in, _IO0::di, _IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
System.Threading.Monitor.Enter(Semaphore);
@@ -322,7 +348,7 @@
:- pragma foreign_proc("C",
try_wait_2(Semaphore::in, Res::out, IO0::di, IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
ML_Semaphore *sem;
@@ -342,7 +368,7 @@
:- pragma foreign_proc("C#",
try_wait_2(Semaphore::in, Res::out, _IO0::di, _IO::uo),
- [promise_pure, will_not_call_mercury, thread_safe],
+ [promise_pure, will_not_call_mercury, thread_safe, tabled_for_io],
"
if (System.Threading.Monitor.TryEnter(Semaphore)) {
if (Semaphore.count > 0) {
@@ -381,22 +407,7 @@
MR_define_entry(mercury__thread__semaphore__nop);
{
- #ifdef MR_DEEP_PROFILING
-
- /*
- ** Perform the actions that would have been taken if we had resumed
- ** back in semaphore.signal and semaphore.wait and left the procedure
- ** normally.
- */
- MR_succip_word = MR_sv(2);
- MR_decr_sp(2);
- MR_np_tailcall_ent(profiling_builtin__det_exit_port_code_sr_3_0);
-
- #else /* !MR_DEEP_PROFILING */
-
MR_proceed();
-
- #endif /* !MR_DEEP_PROFILING */
}
MR_END_MODULE
Index: tests/hard_coded/Mmakefile
===================================================================
RCS file: /home/mercury/mercury1/repository/tests/hard_coded/Mmakefile,v
retrieving revision 1.336
diff -u -r1.336 Mmakefile
--- tests/hard_coded/Mmakefile 17 Sep 2007 07:16:07 -0000 1.336
+++ tests/hard_coded/Mmakefile 18 Oct 2007 00:47:14 -0000
@@ -221,6 +221,8 @@
test_pretty_printer \
test_pretty_printer_defaults \
test_promise_impure_implicit \
+ test_semaphore \
+ test_yield \
tim_qual1 \
time_test \
trace_goal_1 \
Index: tests/hard_coded/test_semaphore.exp
===================================================================
RCS file: tests/hard_coded/test_semaphore.exp
diff -N tests/hard_coded/test_semaphore.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/hard_coded/test_semaphore.exp 17 Oct 2007 06:14:27 -0000
@@ -0,0 +1 @@
+ok
Index: tests/hard_coded/test_semaphore.m
===================================================================
RCS file: tests/hard_coded/test_semaphore.m
diff -N tests/hard_coded/test_semaphore.m
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/hard_coded/test_semaphore.m 18 Oct 2007 01:08:32 -0000
@@ -0,0 +1,27 @@
+% Test semaphore predicates don't crash.
+
+:- module test_semaphore.
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+%-----------------------------------------------------------------------------%
+
+:- implementation.
+
+:- import_module thread.
+:- import_module thread.semaphore.
+
+main(!IO) :-
+ semaphore.new(S, !IO),
+ semaphore.signal(S, !IO),
+ semaphore.signal(S, !IO),
+ semaphore.wait(S, !IO),
+ semaphore.wait(S, !IO),
+ semaphore.signal(S, !IO),
+ io.write_string("ok\n", !IO).
+
+%-----------------------------------------------------------------------------%
+% vi:ft=mercury:ts=8:sts=4:sw=4:et
Index: tests/hard_coded/test_yield.exp
===================================================================
RCS file: tests/hard_coded/test_yield.exp
diff -N tests/hard_coded/test_yield.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/hard_coded/test_yield.exp 18 Oct 2007 00:46:34 -0000
@@ -0,0 +1 @@
+ok
Index: tests/hard_coded/test_yield.m
===================================================================
RCS file: tests/hard_coded/test_yield.m
diff -N tests/hard_coded/test_yield.m
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/hard_coded/test_yield.m 18 Oct 2007 00:57:40 -0000
@@ -0,0 +1,22 @@
+% Test thread.yield does not crash.
+
+:- module test_yield.
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+%-----------------------------------------------------------------------------%
+
+:- implementation.
+
+:- import_module thread.
+
+main(!IO) :-
+ thread.yield(!IO),
+ thread.yield(!IO),
+ io.write_string("ok\n", !IO).
+
+%-----------------------------------------------------------------------------%
+% vi:ft=mercury:ts=8:sts=4:sw=4:et
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to: mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions: mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------
More information about the reviews
mailing list