Skip to content

Handle error in viewflif gracefully#413

Merged
jonsneyers merged 3 commits into
FLIF-hub:masterfrom
UprootStaging:i412
May 10, 2017
Merged

Handle error in viewflif gracefully#413
jonsneyers merged 3 commits into
FLIF-hub:masterfrom
UprootStaging:i412

Conversation

@hrj

@hrj hrj commented May 5, 2017

Copy link
Copy Markdown
Member

This was originally created to address #412
In my system before the fix, viewflif goes into an infinite loop (because d doesn't become NULL). There is no seg fault. After these changes, viewflif closes gracefully.

I thought the seg fault seen by @matthiaskrgr would go away too, but probably that's a different issue (see comments below).

@matthiaskrgr

Copy link
Copy Markdown
Member

I can still reproduce with this changeset, still crashing. :/

@hrj

hrj commented May 7, 2017

Copy link
Copy Markdown
Member Author

@matthiaskrgr Does it crash always or sometimes?

There is a small chance that there is a race condition. But if it is crashing always then there is some other problem, because it is not crashing even once for me, with this change.

Just to be sure, can you do a make clean and then a make viewflif?

@matthiaskrgr

Copy link
Copy Markdown
Member

I freshly cloned the UprootStaging repo and checked out the branch for testing so there is no need for a clean build.

Hmm, it looks like there is a very high probability of crashing on my sys. With the crash command, it only did NOT crash one in 30 times.

@hrj

hrj commented May 7, 2017

Copy link
Copy Markdown
Member Author

@matthiaskrgr Hmm, there is probably some other problem which is compounding with the issue that this PR fixes.

I will remove the closes #412 comment so that the issue remains open, but this PR can be merged.

@matthiaskrgr

Copy link
Copy Markdown
Member

Ok

@jonsneyers jonsneyers merged commit fceb7ff into FLIF-hub:master May 10, 2017
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.

3 participants