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

Add scheme_atexit c function. #1713

Merged
merged 2 commits into from
Jun 4, 2017
Merged

Conversation

LeifAndersen
Copy link
Member

This way programs can actually call atexit. (Otherwise atexit
is frequently not provided in libc as a symbol.)

This way programs can actually call atexit. (Otherwise atexit
is frequently not provided in libc as a symbol.)
@mflatt
Copy link
Member

mflatt commented Jun 4, 2017

Calling atexit() is not always the right thing. Probably the code in scheme_add_atexit_closer() in "thread.c", which sometimes calls atexit() but sometimes does other things, should be moved into scheme_atexit() and called from scheme_add_atexit_closer().

This is because calling atexit is not correct in all
situations, namely on old operating systems.
@LeifAndersen
Copy link
Member Author

Good idea. I have moved the relevant code to scheme_atexit, thanks.

@LeifAndersen
Copy link
Member Author

Would it be a good idea to also bump the version number, or do you think this is too minor for that?

@mflatt
Copy link
Member

mflatt commented Jun 4, 2017

If you need to depend on it from a package, then bump the version number. Otherwise, I'd just leave it.

@LeifAndersen
Copy link
Member Author

Okay thanks. Finally, do you think its worth documenting on: http://docs.racket-lang.org/inside/Custodians.html?q=atexit#%28idx._%28gentag._582._%28lib._scribblings%2Finside%2Finside..scrbl%29%29%29

Otherwise I think its good to merge. (It doesn't seem like there are any tests for thread.c)

@mflatt
Copy link
Member

mflatt commented Jun 4, 2017

I think it's worth documenting, and I agree that it's otherwise ready to go.

@LeifAndersen
Copy link
Member Author

Thanks for your feedback. Merged.

@LeifAndersen LeifAndersen merged commit 5510a76 into racket:master Jun 4, 2017
@LeifAndersen
Copy link
Member Author

(I've also pushed the docs.)

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