Win32 version not properly releasing closed files

[ghost]

I found when I MMDB_close a database, I can't actually delete the file until the process completes. Looking at the code, in the cleanup: code in map_file, based on Microsoft Windows documentation, I believe you should always close the mmh handle (not just on failure). If you successfully get the mmh, and MapViewOfFile to that mmh, then I think their docs say you can close the mmh, since the view will keep the file mapped and locked. Changed lines shown below.

cleanup:;
int saved_errno = errno;

ifdef _WIN32

if (INVALID_HANDLE_VALUE != fd) {
    CloseHandle(fd);
}

if (INVALID_HANDLE_VALUE != mmh) {
CloseHandle(mmh);
}

else

Secondly, looking at the WSAStartup and WSACleanup logic, In MMDB_open, I think you should actually do the WSAStartup if map_file returns successful, and not further down in that function. The MMDB_close() calls WSACleanup() if mmdb->file_content is not NULL, so you should make sure you call WSAStartup any time you have a valid mmdb->file_content, which is as soon as map_file() returns successfully.

I.e. I moved the WSAStartup earlier in MMDB_open...

if (MMDB_SUCCESS != (status = map_file(mmdb)) ) {
    goto cleanup;
}

ifdef _WIN32

WSADATA wsa;
WSAStartup(MAKEWORD(2, 2), &wsa);

endif

Is it easier if I create a pull request to describe this?

I can also email you an updated maxminddb.c file if you would like to look at it.

Thanks.

Bly

[ghost]

I created pull request so that you can see my recommendation. Thanks for your time.

#76

Bly Hostetler

[oschwald]

Thanks for the pull request. You are right on both counts. 👍