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

handle leak on windows #831

Closed
mark-tvu opened this issue Aug 21, 2019 · 9 comments
Closed

handle leak on windows #831

mark-tvu opened this issue Aug 21, 2019 · 9 comments
Labels
Type: Question Questions or things that require clarification

Comments

@mark-tvu
Copy link

Hi guys

   I encountered a handle leak issue on windows, I open , receive, and close repeatedly , although i have already called srt_close and srt_clean, the handle number is always raised.

   I used  the tool "process explore"  to observe handles,  the tool can be download from https://docs.microsoft.com/en-us/sysinternals/downloads/process-explorer.

  My OS platform is win7.  my test SRT version is 1.30 and 1.3.3,  

   Attached is my test code.  

   Could you take a look at this issue
@mark-tvu mark-tvu changed the title handler leak on windows handle leak on windows Aug 21, 2019
@mark-tvu
Copy link
Author

add my test code
test_srt.txt

@maxsharabayko
Copy link
Collaborator

On Windows some tools report leak of handlers on pthreads, when it is statically linked. It is related to a termination code, placed in XPU CRT section by Windows version of pthreads. The code in that section is likely to be optiized when pthreads is linked statically.
If you link pthreads dynamically, everything should be fine.

@mark-tvu
Copy link
Author

mark-tvu commented Aug 22, 2019

Hi Maxlovic,

Thanks for your quick response.

Maybe my test already linked pthreads dynamically instead of statically,

My windows pthread library is from ftp://sourceware.org/pub/pthreads-win32/, i linked pthreads-win32 library from the directory pthreads-win32\2.9.1\lib\x86, under the directory, the .lib files is smaller than the .dll file, so i think these .lib file is for dynamical link. Besides, windows prompts need to load dll file without dll file when running application. lastly, I tried pthreads-win32 version 1.1.0 and version 2.9.1, the issue is reproduced in both version.

Could you guide me how to link pthreads dynamically if i make some mistakes.

PS, I can also use windows task manager to find this handle issue.

@maxsharabayko
Copy link
Collaborator

Besides, windows prompts need to load dll file without dll file when running application.
Then you already have a dynamic link.

Could you please describe what kind of handlers are leaking, at what are you looking at, and all the steps you do?

Brief look at your code, srt_close(fd); should be called inside the while loop, because every time you open a new socket.

@mark-tvu
Copy link
Author

Thanks for your response.

Actually, to not use "goto", my code is "do ....while(0)", so it has no loop.

I am not sure what kind of handlers are leaking. Maybe it is socket, I will go on researching it.

It is easy to reproduce it,

  1. run test code
  2. observer handle in windows "task manager" or in " process explore"

or

  1. make a program using ffmpeg4.1 to call SRT
  2. run this program
  3. observer handle in windows "task manager" or in " process explore"

the tool " process explore" is downloaded from https://docs.microsoft.com/en-us/sysinternals/downloads/process-explorer

1
2
test_srt.zip

@maxsharabayko
Copy link
Collaborator

I mean, your test code should be a bit different.

  • srt_startup() should be called only once at the start, before doing anything SRT-related.
  • srt_socket() creates a new SRT socket every time.
  • srt_cleanup() should be called only once on the process exit.

Also if forget to call freeaddrinfo(ai); if srt_connect returns error, loosing a handler.

The code should look like this:

Click to expand/collapse

using namespace std;

int run_srt(const char*ip, const char * port, int connect_tries, int recv_iterations)
{
	int fd = 0, ret = 0;
	struct addrinfo hints = { 0 }, *ai = NULL, *cur_ai = NULL;

	//struct addrinfo hints = { 0 }, *ai, *cur_ai;
	hints.ai_flags = AI_PASSIVE;
	hints.ai_family = AF_INET;
	hints.ai_socktype = SOCK_DGRAM;

	//hints.ai_family = AF_UNSPEC;
	//hints.ai_socktype = SOCK_DGRAM;
	ret = getaddrinfo(ip, port, &hints, &ai);
	if (ret) {
		printf("error: getaddrinfo returned %d\n", ret);
		return -1;
	}

	cur_ai = ai;

	fd = srt_create_socket();
	if (fd < 0) {
		printf("error: srt_socket %s\n", srt_getlasterror_str());
		return -1;
	}

	ret = -1;
	for (int k = 0; k < connect_tries; k++) {
		// connect to the server, implict bind
		if (SRT_ERROR != srt_connect(fd, cur_ai->ai_addr, cur_ai->ai_addrlen))
		{
			ret = 1;
			break;
		}

		printf("error: srt_connect %s, ip %s, port %s\n",
			srt_getlasterror_str(), ip, port);
		Sleep(1);
	}
	freeaddrinfo(ai);

	if (ret == -1)
	{
		printf("Filed to connect\n");
		srt_close(fd);
		return -1;
	}

	for (int k = 0; k < recv_iterations; k++) {

		char buff[4096];
		ret = srt_recvmsg(fd, buff, sizeof(buff));
		if (SRT_ERROR == ret)
		{
			printf("error: srt_recvmsg %s\n", srt_getlasterror_str());
			ret = -1;
			break;
		}
		printf("succeed to recv msg %d\n", ret);
	};

	srt_close(fd);
	return 0;
}


int main(int argc, char* argv[])
{

	int ret = 0;
	if ((argc != 3) || (0 == atoi(argv[2])))
	{
		printf("usage: test_srt server_ip server_port");
		return -1;
	}

	if (srt_startup() < 0) {
		 printf("error: srt_startup %s\n", srt_getlasterror_str());
		 return -1;
	}

	run_srt(argv[1], argv[2], 1000, 1000);

	srt_cleanup();
}

P.S. You helped to find #833. :)

@maxsharabayko maxsharabayko added the Type: Question Questions or things that require clarification label Aug 23, 2019
@mark-tvu
Copy link
Author

mark-tvu commented Aug 26, 2019

Original use case, we use ffmpeg to call SRT, libsrt_open in ffmpeg includes srt_startup, libsrt_close in ffmpeg includes srt_cleanup, so my test code repeatedly call srt_starup and srt_close to simulate this case.

now I only call srt_startup and srt_cleanup once, i found this issue is disappeared, QA in my company will check it.

Thanks.

@maxsharabayko
Copy link
Collaborator

@mark-tvu Good. Please close this issue once you check it. Or close now and reopen in case the issue persists.

@maxsharabayko
Copy link
Collaborator

@mark-tvy See also PR #496. Might be worth replacing it with the solution, appeared in this discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Question Questions or things that require clarification
Projects
None yet
Development

No branches or pull requests

2 participants