[Mono-dev] New error handling framework for mono

[Rodrigo Kumpera]

On Tue, Aug 18, 2009 at 11:43 AM, Paolo Molaro <lupus at ximian.com> wrote:

> On 08/18/09 Rodrigo Kumpera wrote:
> > > Still open is how this would be integrated on 2.6
>
> I think we should keep this internal for 2.6 and expose it in 2.8 wth
> the other API changes, once we have gained experience with the new API.


Makes sense, otherwise we'll have no opportunity to fix all the eventual
mistakes.
But, for the cases where we want to change an exported function to use it,
simply
adding a _with_error version should be enough while 2.6 is not released.



> > --- /dev/null
> > +++ b/mono/metadata/mono-error.c
> > @@ -0,0 +1,253 @@
> > +
> > +#define mono_error_get_message(E) (((E)->flags &
> MONO_ERROR_ON_SIGNAL_CONTEXT) ? (E)->message : (E)->full_message)
>
> I don't like MONO_ERROR_ON_SIGNAL_CONTEXT: most code is not going to
> know in what mode it was called in. Simply always use the message
> buffer.


I changed this to have a MONO_ERROR_STORE_FULL_MESSAGE flag. My reasoning
behind this is
that some messages are exceptionally large, specially when it does include
the assembly path. Look
at loader.c:330, just the format string is more than 320 chars long and I
don't want to embed it inside
the MonoError machinery.

Now about MONO_ERROR_ON_SIGNAL_CONTEXT, it made sense to me when the caller
did init
MonoError . Now that this change, it has to change.

I've added a MONO_ERROR_DONT_COPY_STRINGS flag but, honestly, I don't think
it is that
helpfull and it doesn't help at all into make it safe for use under signal
context.
I'm not sure if the proposed patch is usable under such conditions since I
could not find any reference
if g_vsnprintf is signal safe or not.


> +void
> > +mono_error_set_assembly_load (MonoError *error, char *assembly_name,
> char *msg_format, ...)
> > +{
> > +     va_list args;
> > +     error->error_code = MONO_EXCEPTION_FILE_NOT_FOUND;
>
> File not found is not the only failure case for an assembly load, we
> should be more flexible here. Think of a bad image, for example, but
> also of an out of memory issue.
>

The current function naming is after the failure code and not the kind of
problem
is represents. So for bad image it's expected that one would use
mono_error_set_bad_image.

I added an option to make it possible to raise any corlib exception, which
should handle
most cases we currently have.

With this mechanism in place, the error_code only needs to encode exceptions
to which
we want to pass additional arguments beyond message.

About out-of-memory situations, yes, this needs special handling as we want
to use the
domains OOM object.

But what should be done when we get an allocation failure on the
mono_exception_set_*
function? Should we just convert such error into an OOM, abort due to a
double-fault or
silently ignore those arguments (and print something on console).


> to keep the code readable. It's kind of an hack.
> An alternative is to have a separate struct with the correct types and
> names and cast to it on public function entry.
>

I spited it into MonoError and MonoErrorInternal.

The attached patch does all suggested changes.

Cheers,
Rodrigo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20090820/c8c2772a/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mono-error-patch.2.diff
Type: text/x-diff
Size: 16801 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20090820/c8c2772a/attachment-0001.bin