Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Tests that no Windows handles are leaked in Directory APIs #99

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Tests that no Windows handles are leaked in Directory APIs #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Corrade/Utility/Directory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ std::vector<std::string> list(const std::string& path, Flags flags) {
WIN32_FIND_DATAW data;
HANDLE hFile = FindFirstFileW(widen(join(path, "*")).data(), &data);
if(hFile == INVALID_HANDLE_VALUE) return list;
Containers::ScopeGuard exit{hFile, FindClose};

/* Explicitly add `.` for compatibility with other systems */
if(!(flags & (Flag::SkipDotAndDotDot|Flag::SkipDirectories))) list.push_back(".");
Expand Down
17 changes: 17 additions & 0 deletions src/Corrade/Utility/Test/DirectoryTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@

#include <clocale>

#ifdef CORRADE_TARGET_WINDOWS
#include <windows.h>
#endif

#include "configure.h"

#ifdef CORRADE_UTILITY_LINUX
Expand Down Expand Up @@ -933,9 +937,22 @@ void DirectoryTest::list() {
"CTest is not able to run XCTest executables properly in the simulator.");
#endif

#ifdef CORRADE_TARGET_WINDOWS
DWORD handleCount = 0;
GetProcessHandleCount(GetCurrentProcess(), &handleCount);
#endif

CORRADE_COMPARE_AS(Directory::list(_testDir),
(std::vector<std::string>{".", "..", "dir", "file"}),
TestSuite::Compare::SortedContainer);

#ifdef CORRADE_TARGET_WINDOWS
/* Ensure we are not leaking handles */
DWORD newHandleCount = 0;
GetProcessHandleCount(GetCurrentProcess(), &newHandleCount);
Copy link
Owner

Choose a reason for hiding this comment

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

Wow, this is cool, didn't even know such thing was possible.

You know what? This would be good to do for everything -- I think this could be doable in setup/teardown routines, but I need to patch TestSuite a bit to allow checks there. Is it okay if I merge just the first part with the scope guard so you have the fix and leave this extra check for later when TestSuite is ready?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can also merge this and refactor it once test suite is ready (or refactor with making it ready).


CORRADE_COMPARE(newHandleCount, handleCount);
#endif
}

void DirectoryTest::listSkipDirectories() {
Expand Down