-
-
Notifications
You must be signed in to change notification settings - Fork 739
Fix segfaults on error on osx. #141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Updated with a fix for all thread related issues on Mac. Do we really not have a single threading unittest in phobos that also calls the GC? I mean this is broken: class MyThread : public Thread {
int run() {
new byte[1024*1024];
return 0;
}
}
int main(char[][] args) {
auto t = new MyThread();
t.start();
t.wait();
return 0;
} Cheers Jakob. |
Bugzilla entry http://d.puremagic.com/issues/show_bug.cgi?id=6290 |
@@ -165,7 +165,8 @@ class Thread | |||
return fp(arg); | |||
else if (dg) | |||
return dg(); | |||
assert(0); | |||
error("neither delegate nor function pointer supplied to run"); | |||
assert(0); // Not reached but compilred doesn't notice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?????? If you want a better message, do
assert(0, "neither delegate nor function pointer supplied to run");
The error
shouldn't be necessary. If it is, there's a bug in the compiler that needs to be fixed, and that should be fixed rather than doing this. If assert(0)
is broken on Mac OS X there are a lot more places than just here which aren't going to work correctly. So, there isn't much point in doing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See updated comment below.
ping? |
Fix a simple typo that lead to segfaults in the GC when spawning another thread. Simple test case: import std.thread; class MyThread : public Thread { int run() { // Tickle the GC new byte[1024*1024]; return 0; } } int main(char[][] args) { auto t = new MyThread(); t.start(); t.wait(); return 0; } Signed-off-by: Jakob Bornecrantz <wallbraker@gmail.com>
Calling assert causes an segfaults on osx. Just do this on all platforms since this is a more helpfull error message. Simple test case: import std.thread; int main(char[][] args) { // User forgets to supply a valid delegate auto t = new Thread(null); t.start(); t.wait(); return 0; } Signed-off-by: Jakob Bornecrantz <wallbraker@gmail.com>
Very minor update to commits, mostly just commit messages. @andralex pong! I'm hoping somebody who actually knows the GC to take a look at the change to it, it works for me. @jmdavis For some reason asserting inside the run() function doesn't work, but this example works: import std.thread;
class Foo
{
int func()
{
assert(false);
}
}
int main(char[][] args)
{
auto f = new Foo();
auto t = new Thread(&f.func);
t.start();
t.wait();
return 0;
} The last patch isn't strictly necessary as it only makes the case in the commit message print a error message and then quite instead of segfaulting. While I prefer the error message I can live without it just as long as I get the GC fix so I can actually use Threads on OSX. Cheers Jakob |
Ping, can somebody with any GC/runtime knowledge take a look at this? |
Sean? |
This is Phobos for D 1.0. I could look at this, but it isn't code I maintain. |
Please do, I really hope this can go into 1.070 I hate to wait another for another release before my users can use threads. Cheers Jakob. |
For what it's worth, the problem with D 1.0 is almost definitely the same as we saw in D 2.0 where we had to switch from using a hardcoded base stack address to obtaining it via an OS call. |
@complexmath Okay, well the patch I supplied fixed the problem I was seeing. I think this is unrelated to the issue you mentioned IIRC I asked on that thread and Walter said that no fix was needed for D1. Cheers Jakob. |
Interesting. Well, if this change fixes D1 for you then that's good enough for me. I think it's too late to get this into the upcoming release, but it can be merged immediately after. |
Fix segfaults on error on osx.
Calling assert causes an segfaults on osx. Just do this on
all platforms since this is a more helpfull error message.