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

[MacOS] Player crashes when seeking a folder with no permissions to access. #1476

Closed
elsemieni opened this Issue Nov 4, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@elsemieni
Copy link
Member

elsemieni commented Nov 4, 2018

Player platform:

MacOS (possibly others?...). (Tested in High Sierra 10.13.1 with EasyRPG 0.5.4.).

Describe the issue in detail and how to reproduce it:

When you execute player and while it's seeking folders in search of games (in splash logo), if you have a folder in the same directory that player can not access (Permission denied), it crashes. This could happen when you have a folder without enough previleges to access, or if you run Player as a part of an App Bundle (it reads folders from root?).

I tried to debug with the included stack trace that MacOS throws me, suspecting it crashes when it returns from function FileFinder::GetDirectoryMembers.

The crash is related by this line: std::shared_ptr< ::DIR> dir(::opendir(wpath.c_str()), ::closedir); since making isolated test cases (avoiding the problematic folders before entering this line and commenting it, making player unable to read any projects) it don't make any crash.

Stack trace (Player tries to open "./.fseventsd", log prints Permission Denied ).

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_pthread.dylib       	0x00007fff52d043d5 pthread_mutex_lock + 0
1   libsystem_c.dylib             	0x00007fff52af04bc closedir + 25
2   easyrpg-player                	0x0000000104b08cfa std::__1::__shared_ptr_pointer<DIR*, int (*)(DIR*), std::__1::allocator<DIR> >::__on_zero_shared() + 106 (memory:3795)
3   libc++.1.dylib                	0x00007fff50aeb8fd std::__1::__shared_weak_count::__release_shared() + 43
4   easyrpg-player                	0x0000000104a6f70c std::__1::shared_ptr<Mix_Chunk>::~shared_ptr() + 44 (memory:4627)
5   easyrpg-player                	0x0000000104a6a625 std::__1::shared_ptr<_Mix_Music>::~shared_ptr() + 21 (memory:4627)
6   easyrpg-player                	0x0000000104ae0429 FileFinder::GetDirectoryMembers(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, FileFinder::Mode, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 18425 (filefinder.cpp:826)
7   easyrpg-player                	0x0000000104ae2199 FileFinder::CreateDirectoryTree(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool) + 473 (filefinder.cpp:255)
8   easyrpg-player                	0x0000000104e3e015 Window_GameList::Refresh() + 2053 (window_gamelist.cpp:37)
9   easyrpg-player                	0x0000000104d9ad44 Scene_GameBrowser::CreateWindows() + 3924 (scene_gamebrowser.cpp:98)
10  easyrpg-player                	0x0000000104d99dbc Scene_GameBrowser::Start() + 204 (scene_gamebrowser.cpp:46)
11  easyrpg-player                	0x0000000104d868e9 Scene::MainFunction() + 137 (scene.cpp:79)
12  easyrpg-player                	0x0000000104c29a9c Player::MainLoop() + 44 (player.cpp:225)
13  easyrpg-player                	0x0000000104c2992a Player::Run() + 186 (player.cpp:219)
14  easyrpg-player                	0x00000001049b5117 main + 39 (main.cpp:28)
15  libdyld.dylib                 	0x00007fff52a7c145 start + 1

PD: Now reading the stack trace quitetly and after investigate what shared_ptr does, looks like closedir don't like the case when open failed... Maybe...

@carstene1ns

This comment has been minimized.

Copy link
Member

carstene1ns commented Nov 4, 2018

Thanks, will try the same scenario under GNU/Linux and investigate from there.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 4, 2018

The Windows version should not be affected by this because when I wrote the Game Browser I added error handling to get rid of warning spam when trying to read folders like "System Volume Informaton".
(or is a regression)

@elsemieni

This comment has been minimized.

Copy link
Member Author

elsemieni commented Nov 4, 2018

Okay, I made some little tests and this bug is not happening at GNU/Linux.

Today I compiled Player with debug symbols and figured that when it crashes (when returning from FileFinder::GetDirectoryMembers ) dir is NULL. So my theory that shared_ptr that tries to close an null folder instance is the source of the problem gains more suspect.

Here's a test case:

#include <dirent.h>

int main(int c, char * v[]) {
	int len;
	struct dirent * pDirent;
	DIR * pDir;

	if (c < 2) {
		printf("Usage: testprog <dirname>\n");
		return 1;
	}
	pDir = opendir(v[1]);
	if (pDir == NULL) {
		printf("Cannot open directory '%s'\n", v[1]);
		closedir(pDir); // this is the test case !!!!!!!!!!!!!!!!
	}

	closedir(pDir);
	return 0;
}

And tested it with a root-owned folder with 700 permissions. (In windows tested with C:/ProgramData folder instead). In all cases opendir returns NULL but the behaivour of closedir was different:

On windows: closedir(pDir) with pDir = NULL does nothing.
On MacOS: closedir(pDir) with pDir = NULL crashes.
On GNU/Linux (ubuntu): closedir(pDir) with pDir = NULL does nothing.
Other platforms: untested.

Sounds like standard libs in OSX does not handle well this case (or maybe it's just luck, I dunno... ).

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 4, 2018

We use a shared_ptr with custom deleter and I guess the problem is that the deleter is also being called when the shared_ptr is nullptr.

I don't want to make gigantic changes to filefinder as this code has many modifcations in my VFS branch and rebasing this is a pain.

Working solution:

-std::shared_ptr< ::DIR> dir(::opendir(wpath.c_str()), ::closedir);
+std::shared_ptr< ::DIR> dir(::opendir(wpath.c_str()), [](::DIR* d) { if (d) ::closedir(d); });

Ghabry added a commit to Ghabry/easyrpg-player that referenced this issue Nov 4, 2018

elsemieni added a commit to elsemieni/easyrpg-player-netherware that referenced this issue Nov 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.