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

Fix fdc2e85: Double close of file handles #7185

Merged
merged 1 commit into from Feb 6, 2019

Conversation

nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Feb 6, 2019

When unpacking downloaded content, the downloaded .gz file was being opened with fopen, the OS file handle given to zlib, and then afterwards zlib told to close the file.

But the FILE * object was never closed with fclose, meaning the stdio library would have a hanging file object, whose file handle was now invalid or referred to a different file. This caused asserts during shutdown with Microsoft's C library in debug mode.

Fix this by properly duplicating the OS handle and fcloseing the FILE * object, before giving the handle to zlib.

@PeterN
Copy link
Member

PeterN commented Feb 6, 2019

Is this not enough?

-       gzFile fin = gzdopen(fileno(ftmp), "rb");
+       gzFile fin = gzdopen(dup(fileno(ftmp)), "rb");

@nielsmh
Copy link
Contributor Author

nielsmh commented Feb 6, 2019

At the very least the "else if" at the bottom would also need changing so ftmp is properly closed.

And apparently dup() is too POSIX for Microsoft, and _dup() is too ANSI for everyone else?

@LordAro
Copy link
Member

LordAro commented Feb 6, 2019

Concur with @PeterN , the anonymous lambda is a bit too dirty for my tastes

@PeterN
Copy link
Member

PeterN commented Feb 6, 2019

Looks okay to me and is exactly what the zlib documentation says to do.

@nielsmh nielsmh merged commit db2c0cc into OpenTTD:master Feb 6, 2019
@nielsmh nielsmh deleted the fix-double-close branch February 6, 2019 20:09
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

3 participants