[m-rev.] for review: fix handling of contexts in engines

[Peter Wang]

On 2007-04-13, Zoltan Somogyi <zs at csse.unimelb.edu.au> wrote:
> For review by Peter Wang.
> 
> Zoltan.
> 
> Fix some software rot that prevented I/O operations from working in mmos
> grades. The problem was the change to the I/O module to make it use thread
> local storage via a new field of the MR_Context structure which was accessed
> via the MR_eng_this_context field of the engine, instead of via the
> MR_eng_context field. The new field was not set by the code for initializing
> the contexts used by own stack minimal model tabling.
> 
> runtime/mercury_context.h:
> runtime/mercury_engine.h:
> 	Add significant new documentation about how fields of the MR_Context
> 	structure are accessed, both because the documentation is useful and to
> 	make similar mistakes less likely in future.

Thanks a lot for that.

> 
> 	Add a macro for use by own stack minimal model tabling.
> 
> runtime/mercury_thread.c:
> 	Add a comment about a link to mercury_engine.h.
> 
> runtime/mercury_thread.h:
> 	Convert to four-space indentation, and fix some formatting.
> 
> runtime/mercury_mm_own_stacks.c:
> 	Add code for filling in the missing fields of newly created contexts.
> 
> runtime/mercury_wrapper.c:
> 	In own stack minimal model grades, set up the main context properly.
> 	The previous code was based on a flawed understanding of the
> 	relationalship between MR_eng_context and MR_eng_this_context.
> 
> tests/debugger/mmos_print.{m,inp,exp}:
> 	Add a new test case (which we don't yet pass due to a problem with
> 	formatting of mdb output) to test the fix. The old versions of the
> 	compiler don't pass this test case, because the "p *" commands of the
> 	debugger invoke I/O code in the Mercury standard library, which fails
> 	with a segfault due to the thread local fields of generators' contexts
> 	being unitialized.

uninitialized.

> @@ -647,6 +693,16 @@
>          MR_save_hp_in_context(save_context_c);                                \
>      } while (0)
>  
> +#define MR_copy_eng_this_context_fields(to_cptr, from_cptr)                   \
> +    do {                                                                      \
> +        to_cptr->MR_ctxt_resume = from_cptr->MR_ctxt_resume;                  \
> +        to_cptr->MR_ctxt_thread_local_mutables =                              \
> +            from_cptr->MR_ctxt_thread_local_mutables;                         \
> +        /* it wouldn't be appropriate to copy the spark_stack field */        \
> +        /* it wouldn't be appropriate to copy the saved_owners field */       \
> +    } while (0)

Maybe add a comment that this is for own stack minimal model tabling.
In general, copying the MR_ctxt_resume would not be useful.

Also, I don't know anything about minimal model tabling, but would it be
more appropriate to clone the thread local mutable array here
(using MR_clone_thread_local_mutables)?

> Index: runtime/mercury_mm_own_stacks.c
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_mm_own_stacks.c,v
> retrieving revision 1.10
> diff -u -b -r1.10 mercury_mm_own_stacks.c
> --- runtime/mercury_mm_own_stacks.c	18 Feb 2007 08:01:56 -0000	1.10
> +++ runtime/mercury_mm_own_stacks.c	13 Apr 2007 08:18:55 -0000
> @@ -556,6 +556,14 @@
>          MR_next_gen_context++;
>          ctxt = MR_create_context(strdup(buf), MR_CONTEXT_SIZE_SMALL,
>              generator);
> +        MR_copy_eng_this_context_fields(ctxt, MR_ENGINE(MR_eng_this_context));

> +        ctxt->MR_ctxt_next = NULL;
> +#ifdef  MR_THREAD_SAFE
> +        ctxt->MR_ctxt_resume_owner_thread = NULL;
> +        ctxt->MR_ctxt_resume_c_depth = 0;
> +        ctxt->MR_ctxt_saved_owners = NULL;
> +#endif
> +        ctxt->MR_ctxt_spark_stack = NULL;

These fields are initialised by MR_create_context().

I noticed that the only caller of MR_get_context_for_gen() immediately
overwrites the `MR_ctxt_resume' field.  So copying the `MR_ctxt_resume'
field in `MR_copy_eng_this_context_fields' is unnecessary.

Peter
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------