Skip to content
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

Call llvm_shutdown() #1577

Merged
merged 7 commits into from Mar 1, 2012
Merged

Call llvm_shutdown() #1577

merged 7 commits into from Mar 1, 2012

Conversation

ryoqun
Copy link
Member

@ryoqun ryoqun commented Feb 28, 2012

According to LLVM docs(http://llvm.org/docs/ProgrammersManual.html#shutdown), calling llvm::llvm_shutdown() after use of LLVM is advised to prevent memory leaks.

Thus, rubinius should call llvm_shutdown() immediately before exiting from main() in the CLI driver.

@dbussink
Copy link
Contributor

There's is also Environment::halt_and_exit that probably also needs a similar change. This is used when another thread than the main thread exits.

@ryoqun
Copy link
Member Author

ryoqun commented Feb 29, 2012

Thank you for your suggestion! I'm currently trying to address it. On the course of it, I found a bug in my modification. The above commits are for that.. Sorry..

@ryoqun
Copy link
Member Author

ryoqun commented Mar 1, 2012

I made llvm_shutdown() be called from Environment::halt_and_exit, too!

env.load_conf(path);
int exit_code = 0;

{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for this additional indentation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for destructing env before calling llvm_shutdown().

Rubinius SEGV-ed without this indentation when exiting with unusual conditions (interrupt by Ctrl-C from the console). So I guessed Environment destructor is touching llvm objects/functions after llvm_shutdown() is called.

For example, without this indentation, I couldn't clean-build rubinius.

Should I explicitly mention this indentation purpose? Or, Should I put the whole indented code into another function? Is it better?

Thanks for reviewing!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's a good one :). A comment to describe this case is probably fine for now, so we know why it's there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe. Thanks! I'll push a commit to add that comment shortly!

@ryoqun
Copy link
Member Author

ryoqun commented Mar 1, 2012

As I said I added a comment to the explicit indentation!

dbussink added a commit that referenced this pull request Mar 1, 2012
@dbussink dbussink merged commit cc07556 into rubinius:master Mar 1, 2012
@ryoqun
Copy link
Member Author

ryoqun commented Mar 1, 2012

Thanks a lot :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants