Download Reference Manual
The Developer's Library for D
About Wiki Forums Source Search Contact

Ticket #750 (closed enhancement: fixed)

Opened 16 years ago

Last modified 16 years ago

Make the standard Exception have the trace feature

Reported by: keinfarbton Assigned to: sean
Priority: normal Milestone: 0.99.5
Component: Tango Version: 0.99.3 Triller
Keywords: Cc:

Description

Much code is throwing simply Exception. This is bad, because actually it does not have the trace feature. But there are only few cases where we don't want the trace. I only know of the OutOfMemory.

So I think, it isn't a good approach to change everything to TracedException. Instead the Exception should have the trace and there should be a NotTracedException as a base for OutOfMemory.

Change History

11/16/07 00:28:47 changed by kris

  • status changed from new to closed.
  • resolution set to wontfix.

Instead, the code should be changed to throw a realistic exception. It is appallingly bad practice to just throw Exception ... next thing, people will be just throwing Object instead :p

We can't derive Exception from Traced anyway, Frank, since OutOfMemoryException? needs to bypass any further attempt to allocate memory (for example).

Sorry

11/16/07 00:44:05 changed by keinfarbton

I think, it is not a good approach to define what is a bad habit. If a library throws Exception, its the user of the lib, who does not have the trace. E.g. tango itself throws Exception, and its me not having the trace. I think (beside technical problems) it would be good to let Exception have the trace and make a special case for the special case: OutOfMemoryException, which is very special.

Probably Exception can have a ctor with a marker argument. This ctor can be called from the derived OutOfMemoryException and so it can bypass all Tracing stuff.

11/16/07 00:55:13 changed by sean

Right now, Exception in Tango is equivalent to Throwable in Java. Everything that is thrown is expected to derive from Exception. This is relevant because the runtime actually expects to be dealing with Exception objects in some cases.

I also believe it is bad form to catch and swallow Exception objects unless the user /really/ knows what they're doing. An OutOfMemoryException? derives from Exception, as does AssertException?, etc. All of these are typically a critical failure of the application, and should not typically be recovered from. So in short, while I'm fine with adding trace features to Exception, another higher-level object would have to replace it in the hierarchy, let's say Throwable. Then the runtime would expect instances of Throwable, and OutOfMemoryException? would derive from Throwable instead of from Exception, etc.

But this seems to me like it would return us to the exact situation we have now, unless the point that people naturally want to deal with Exception objects, so by making the top-level name something else we would be reducing the change the user would accidentally ignore something important?

11/16/07 01:04:13 changed by kris

Yeah, it's a circular game. Where does tango throw Exception, Frank? That needs to be fixed ...

11/16/07 01:16:06 changed by keinfarbton

grep -rn "new Exception\\>"

gives about 60 matches. This does probably not cover all.

But this is not my point. We can fix that, but we cannot make sure, anyone will not use Exception. And even if it does not make any sense to recover, I want to have a trace. Debugging D is so bad, not having a trace is a disaster, if only locating the problematic place take a long time.

11/16/07 01:22:16 changed by kris

That's truly shocking, and will be rectified asap.

Also, we're not 'defining' what a bad habit is ... the entire industry knows that tossing Throwable (for example) is circumventing the intended purpose of exceptions. So, it's not even a question of education per se. We can't resolve stupidity, Frank

11/16/07 01:34:26 changed by keinfarbton

This might be true. But I am pretty sure, many of the existing D libs will also use Exceptions, e.g. mango, flectioned, duit. I think the name matters.

BTW there are also matches for

grep -rn "catch.\+\\<Exception\\>"

11/16/07 01:58:19 changed by kris

You may be right. If so, then we should do like Sean says and make Throwable the lowest level. Catching or throwing a Throwable would become a hanging offence. It would also make Tango further detached from Phobos (as an fyi)

BTW, where are all these cases in Tango? I don't have access to the source right now

11/16/07 03:56:59 changed by kris

ok, all the user-level instances of new Exception are now resolved in Tango

11/16/07 08:40:32 changed by larsivi

  • milestone set to 0.99.4.

I see this got closed early on - is that correct? The ticket don't look entirely resolved yet. I'm tempted to agree with Frank on the basic premise of the request. However, considering that the Exception hierarchy is something that is part of the Tango/Phobos differences (and will make them even more different, Phobos currently have no notion of traceable), we should step carefully.

12/18/07 02:08:32 changed by kris

  • status changed from closed to reopened.
  • resolution deleted.

12/18/07 02:09:21 changed by kris

  • status changed from reopened to new.
  • owner changed from kris to sean.

Seems like part of this will require a runtime hook, so assigning to Sean

12/21/07 14:12:41 changed by larsivi

  • milestone changed from 0.99.4 to 0.99.5.

01/31/08 04:28:53 changed by sean

  • status changed from new to assigned.

This fix requires some changes to flectioned if you plan to use that, so I've placed an updated copy here temporarily:

http://www.invisibleduck.org/~sean/tmp/flectioned.d

It should also be noted that attaching flectioned causes a noticeable performance hit. Using this test app:

void fnA()
{
    fnB();
}

void fnB()
{
    fnC();
}

void fnC()
{
    assert( false );
}

void main()
{
    fnA();
}

A quick test shows this for a run without flectioned linked:

real    0m0.063s
user    0m0.016s
sys     0m0.031s

And this is with flectioned linked (and thus tracing enabled):

real    0m0.313s
user    0m0.156s
sys     0m0.109s

This obviously isn't an exacting test, but it's worth noting. I think the timing could be improved with a more optimized trace mechanism, but a production app may consider leaving tracing out altogether if exceptions are a common occurrence.

01/31/08 07:24:27 changed by keinfarbton

At startup, flectioned loads all symbols. This cost time and heap. At building the stacktrace some more time is consumed, hopefully this is not relevant.

But does it cost time/heap on normal program execution after startup?

01/31/08 20:11:03 changed by sean

  • status changed from assigned to closed.
  • resolution set to fixed.

Fixed an off-by-one error in the shuffle algorithm. Turns out I'd unintentionally implemented a variant of Sattolo's algorithm for producing cyclic permutations. Also fixed modulo bias in the default random routine for shuffle.* Moved exception trace generation from tango.core.Exception into the runtime. This involved adding a TraceInfo? interface to the global Exception class, and moving the machinery to generate a trace into genobj.d.

* The new hook to attach a trace hander is as follows:

extern (C) void  rt_setTraceHandler( void function( void* ptr = null ) h );

This feature is exposed in tango.core.Runtime as the new 'traceHandler' property.

* The change to tracing inspired me to similarly rewire the collection handler code to move that hook into the runtime as well. This was done to improve efficiency for collections, as each collected object can now simply test a pointer to see if a handler is set rather than calling into user-space to perform the same check. The new hook for this mechanism is:

extern (C) void  rt_setCollectHandler( void function(Object) h );

And the feature is also exposed in the Runtime object as the new collectHandler property. I thought about the name 'collectHandler' being in Runtime rather than GC, but I think the accompanying explanation for the function justifies the decision.

These changes are in [3145].