magic-lang / ooc-kean

ooc port of parts of Kean.
MIT License
7 stars 30 forks source link

Exception StackFrames are not per thread #1649

Open ds84182 opened 8 years ago

ds84182 commented 8 years ago

Example code:

use concurrent
use system

threadA: func {
    Time sleepSec(5)
    raise("Exception from ThreadA")
}

threadB: func {
    try {
        Time sleepSec(6)
    } catch (ex: Exception) {
        "Handling exception on ThreadB" println()
        ex print()
    }
}

main: func {
    ta := Thread new(threadA)
    tb := Thread new(threadB)
    ta start()
    tb start()
    ta wait()
    tb wait()
    ta free()
    tb free()
}

Possible output:

Handling exception on ThreadB
[Exception]: Exception from ThreadA
[fancy backtrace]
0     BacktraceHandler backtrace()  in Backtrace          (at /home/dwayne/magic-lang-stuff/ooc-kean/source/system/Backtrace.ooc:26)      
1     Exception addBacktrace()      in Exception          (at /home/dwayne/magic-lang-stuff/ooc-kean/source/system/Exception.ooc:113)     
2     Exception throw()             in Exception          (at /home/dwayne/magic-lang-stuff/ooc-kean/source/system/Exception.ooc:138)     
3     raise_withClass()             in Exception          (at /home/dwayne/magic-lang-stuff/ooc-kean/source/system/Exception.ooc:86)      
4     threadA()                     in trycatchthreadbug  (at /home/dwayne/magic-lang-stuff/threadbug/trycatchthreadbug.ooc:7)            
5     start_thread()                in                    (at /build/buildd/glibc-2.21/nptl/pthread_create.c:333)                         
6     clone()                       in                    (at /build/buildd/glibc-2.21/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:111)
(SIGSEGV) segmentation fault
[fancy backtrace]
0     BacktraceHandler backtrace()     in Backtrace  (at /home/dwayne/magic-lang-stuff/ooc-kean/source/system/Backtrace.ooc:26) 
1     Exception getCurrentBacktrace()  in Exception  (at /home/dwayne/magic-lang-stuff/ooc-kean/source/system/Exception.ooc:162)
2     _signalHandler()                 in Exception  (at /home/dwayne/magic-lang-stuff/ooc-kean/source/system/Exception.ooc:226)
3     killpg()                         in            (at (null):0)              
ghost commented 8 years ago

Thanks for reporting, we are aware of this. In fact we changed the use of ThreadLocal in Exception in order to minimize dependencies between modules few months ago. We just try to not use exceptions between threads (in fact we were trying to get rid of them completely at some point). @marcusnaslund anything to add ?

marcusnaslund commented 8 years ago

we changed the use of ThreadLocal

In other words, we threw it out the window. We generally use little-to-no exceptions in this repo, so while this technically is a bug, we haven't really bothered. If there's a nice way of fixing it that doesn't involve reinstating ThreadLocal, I'm all ears.

marcusnaslund commented 8 years ago

(Hmm, does system/external, system/Mutex.ooc and system/structs/HashMap.ooc have everything we need to use a make-shift ThreadLocal without the dependency problems we had before?)

ds84182 commented 7 years ago

So I thought about this some today. Since magic doesn't use GC exceptions are leak landmines. Instead of using a setjmp/longjmp based exception system, we could switch to something like this:

funcThatThrows: func throws {
  someBuffer := Buffer new~allocate(1024)

  // (This or we can totally annotate a variable as scoped with some awesome auto-free code)
  scope_exit {
    someBuffer free()
  }

  // some later checking
  if (everythingIsBroken) raise("Oh no!");
}

funcThatDoesntThrow: func {
  // Calling a function that throws inside a function that doesn't throw without try catch
  // is a runtime error.
  // This could also lead straight to an assert+exit at runtime (if you don't include the try catch).
  try {
    funcThatThrows()
  } catch (ex: Exception) { /* handle ex */ }
}

On the C side of things we pass in an extra parameter that gets filled with an exception pointer. It's been a while since I've looked at OOC's compiled code so I'll just wing it.

void funcThatThrows(Exception *_ex) {
  // skip right to the exception throwing...
  _ex = WhatEverYouCallToCreateANewExceptionObject(WithTheArgsHere);
  // scope_exit stuff
  someBuffer->free(); // or something like this
  return;
}

// Yadda yadda

Also, a thing I did not address above was calling a func that will throw inside a func that will throw. In that case the error will be propagated and scope_exit will run, unless try catch'd.

Overall, this would require some changes to the language:

ghost commented 7 years ago

@thomasfanell

thomasfanell commented 7 years ago

@ds84182 I'll take a look at this when I've met my current deadlines.

alexnask commented 7 years ago

@ds84182

Exceptions

Regarding your "throws" proposal, I would personally prefer the keyword to be "maythrow" or something similar (although that is purely stylistic, not really useful input).

Calling a maythrow function from an nothrow function (un-annotated function) without catching it should be a compile time error, it does not need to be a runtime error.

However, this means lots and lots of maythrow annotations will need to be added to existing magic code.
A better strategy could be the following:

This way, the specification is a hint for people reading the code and the compiler to warn you if you want some specific behavior.
This allows for existing code to remain valid and have annotations be added gradually rather than a massive refactor upfront.

Exceptions would have to be lifted as a language construct, as you said (although this could be done without breaking anything but the Exception base class by adding some kind of builtin for throwing instead of a throw keyword and using it in the throw method).

EDIT: Also, if exceptions become a language feature, it would be nice to have them be ABI compatible with C++ exceptions and be able to at least catch (obviously with no type information) exceptions from extern C++ functions.
I don't know much about how difficult this would be to implement to be honest, especially in a portable way.

Adding a pointer to some exception information and filling it in like you show in your pseudocode would work but is not a clean solution (imo).
The current implementation is probably fine, just has to be tied to the compiler/codegen through some new keywords.
Besides, this would break ABI compatibility with C code (for example, you will not be able to use functions that throw exceptions as the callables of threads and many other FFI examples).

Scope guards

Scope guards would be really nice to have indeed, especially in magic where the GC is turned off.
If we do go with scope guards, it would be nice to have success and failure guards in addition to an exit guard, like Dlang.

Essentially, the exit guard is executed any time you leave a scope, the success guard is executed when exiting a scope but not through an exception and the failure guard is executed when exiting a scope through an exception (but does not catch/rethrows the exception).

I have a feeling that the codegen for these would be a little tricky, since the backend currently has a one to one mapping with the resulting C code.
It would probably have to be implemented by adding a stack of guards to every scope, attaching them when building the AST as well as adding a GuardRef node of some kind where the scope guard appears, resolving them after resolving the body of the scope and popping + generating code in the backend.

alexnask commented 7 years ago

Annotating variables as scoped and auto calling free() on them would be really nice too but requires some escape analysis (which would also allow for allocating objects on the stack but is probably really difficult to implement in rock).

EDIT: I do agree the combination of those features + a fixed version of rock#68 would be a massive improvement for writing GC-free code.
Templated functions and template parameter deduction (currently only works with generics) would be pretty nice too but all these features require work and time.

alexnask commented 7 years ago

One potential issue with the maythrow/nothrow annotations is what happens with function pointers / closures.
Do we assume calling a function pointer may always throw or do we also add nothrow to the type system?
(I'm leaning towards assuming the call may throw myself but I can understand arguments for the second solution too)

ds84182 commented 7 years ago

I'd argue that function pointers by default don't throw since much of the code in here already assumes that they don't throw. A throw specifier on a function pointer would tell the compiler to call the function with the extra exception argument. Doing it the other way around would complicate C interoperability because a regular C function as a function pointer wouldn't take an exception as an argument.

As for maythrow and nothrow that seems fine for now, but I feel that the complier going through functions trying to figure out if they throw or not could get expensive fast.

On Wed, Nov 16, 2016, 6:25 AM Alexandros Naskos notifications@github.com wrote:

One potential issue with the maythrow/nothrow annotations is what happens with function pointers / closures.

Do we assume calling a function pointer may always throw or do we also add nothrow to the type system?

(I'm leaning towards assuming the call may throw myself but I can understand arguments for the second solution too)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/magic-lang/ooc-kean/issues/1649#issuecomment-260922431, or mute the thread https://github.com/notifications/unsubscribe-auth/ACKbZe-WjVg6EuOX_G91ZxyzdgdnoX-Hks5q-ugwgaJpZM4IG8Ht .

alexnask commented 7 years ago

@ds84182

Even using just a throws attribute, the compiler would have to check that non-throw functions catch exceptions from every throwing function they call to error out otherwise. I don't think it would be really expensive to do the checking, essentially the whole call graph can be checked in one pass if you start by checking leaf functions. I'm sure there are some edgecases to consider but being familiar with the rock internals I would say that this check is the least of our worries efficiency wise.

As I said, I'm not too keen on using an extra argument for exceptions, imho they should behave like they do in other native languages with built in support (and as they do in ooc/magic right now, except as a language feature), so your point on interop is not that relevant in that case.

Actually, using a pointer to an exception would hurt interop the other way (passing ooc functions to C), since the C side has no way of knowing it must pass a special "exception" structure to the ooc side, making it impossible to pass ooc functions that may throw to C code.

In my opinion, function pointers and closures should be treated as throwing by default (for obvious safety reasons).
With the system I outlined, this would mean that if you want your higher order function to be nothrow, you would need to try-catch calls to the function arguments, which makes sense to me, but you could still write your regular code with no nothrow attribute (or the throw/maythrow attribute) and no try/catch blocks.