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

Make AtExit a stack and make DirectoryTemporary more robust #2856

Merged
merged 2 commits into from
Sep 22, 2018

Conversation

ChrisJefferson
Copy link
Contributor

This is kind of 2 PRs merged together, can split if required.

Firstly, the error message when cleaning up a DirectoryTemporary if it was already deleted wasn't useful, and other temporary directories wouldn't be deleted after the first failing one. This could easily occur if a temporary directory was made, then IO_fork was called.

Secondly, while debugging this I was having some problems which turned out to be caused by the fact the AtExit functions are run in-order. This is against how exit handlers normally work, as it means the first handlers GAP installs (which do things like flush buffers) get run before later, or user-installed handlers.

I pop the list of AtExit handlers, rather than just calling Reverse on the list, in case new handlers are installed as GAP is closing.

@ChrisJefferson ChrisJefferson added topic: library release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes topic: error handling labels Sep 21, 2018
if IsFunction(f) then
CALL_WITH_CATCH(f,[]); # really should be CALL_WITH_CATCH here
CALL_WITH_CATCH(f,[]);
Copy link
Member

Choose a reason for hiding this comment

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

Hehe, nice comment :)

@codecov
Copy link

codecov bot commented Sep 21, 2018

Codecov Report

Merging #2856 into master will decrease coverage by <.01%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master    #2856      +/-   ##
==========================================
- Coverage    76.1%   76.09%   -0.01%     
==========================================
  Files         480      480              
  Lines      240949   240955       +6     
==========================================
- Hits       183363   183362       -1     
- Misses      57586    57593       +7
Impacted Files Coverage Δ
lib/session.g 59.25% <100%> (+0.76%) ⬆️
lib/files.gd 63.63% <75%> (+0.09%) ⬆️
hpcgap/lib/hpc/stdtasks.g 63.42% <0%> (-1.54%) ⬇️
src/iostream.c 62.35% <0%> (-1.15%) ⬇️
src/gap.c 84.2% <0%> (-0.15%) ⬇️
lib/package.gi 71.17% <0%> (+0.02%) ⬆️
src/listfunc.c 94.31% <0%> (+0.17%) ⬆️
src/streams.c 77.64% <0%> (+0.39%) ⬆️

@fingolfin fingolfin merged commit 13e043b into gap-system:master Sep 22, 2018
@ChrisJefferson ChrisJefferson deleted the tempdir branch January 20, 2019 13:38
@fingolfin fingolfin changed the title Make AtExit a stack and handle DirectoryTemporary better Make AtExit a stack and make DirectoryTemporary more robust Aug 22, 2019
@fingolfin fingolfin added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 22, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes topic: error handling topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants