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

Memory leak under Windows #49

Closed
jswirl opened this issue Feb 4, 2016 · 12 comments
Closed

Memory leak under Windows #49

jswirl opened this issue Feb 4, 2016 · 12 comments

Comments

@jswirl
Copy link

jswirl commented Feb 4, 2016

usrsctptest4
This is after applying this PR: #48
I'm not really familiar with Windows, so I could be doing something wrong, but it appears there may be leaks some place else.

The Windows UMDH utility seems to point to some suspicious parts in sctp_init_ifns_for_vrf():
https://github.com/sctplab/usrsctp/blob/master/usrsctplib/netinet/sctp_bsd_addr.c#L356-L362 and
https://github.com/sctplab/usrsctp/blob/master/usrsctplib/netinet/sctp_bsd_addr.c#L404-L409.

Here's the UMDH log from the test program I used:
stackdiff.txt

@tuexen
Copy link
Member

tuexen commented Feb 11, 2016

OK. Can you retest with the current code? If the problem persists, could you share your test program?

@Globik
Copy link

Globik commented Feb 11, 2016

Аnd I am on windows with msys2. I cant find netinet6 with headers files. What is the name of such library for netinet6?

@tuexen
Copy link
Member

tuexen commented Feb 11, 2016

I guess the files are in
https://github.com/sctplab/usrsctp/tree/master/usrsctplib/netinet6
You should be able to build the userland stack with nmake or cmake.

@tuexen
Copy link
Member

tuexen commented Feb 12, 2016

OK, found the test program. Wasn't reading the screenshot... We will test it.

@tuexen
Copy link
Member

tuexen commented Feb 12, 2016

I added a simple test program test_libmgmt.c and it doesn't show any leaks on Linux. We will test it on Windows next week.

Please note that you must use

while (usrsctp_finish() != 0) {
#ifdef _WIN32
    Sleep(1000);
#else
    sleep(1);
#endif
}

instead of

usrsctp_finish();

in your test program. If usrsctp_finish() does return a non-zero value, the stack is not cleaned up and you might leak memory when just calling usrsctp_init() again.

@jswirl
Copy link
Author

jswirl commented Feb 13, 2016

Okay. I don't have access to my Windows machine now, but I'll test it on Monday and let you know.
Thanks for your help.

@tuexen
Copy link
Member

tuexen commented Feb 14, 2016

I now see where the leak occurs on Windows. It doesn't occur on other platforms. Need to think about a correct way of fixing it. Will let you know when I think the issue is fixed.

@jswirl
Copy link
Author

jswirl commented Feb 15, 2016

I can confirm that there are still leaks using the most recent code, this time I waited for usrsctp_finish() to return properly before calling usrsctp_init()
usrsctptest5

@jswirl
Copy link
Author

jswirl commented Feb 15, 2016

@tuexen: just out of curiosity, where is the leak that you said you saw?
thanks.

@tuexen
Copy link
Member

tuexen commented Feb 15, 2016

The allocations in
https://github.com/sctplab/usrsctp/blob/master/usrsctplib/netinet/sctp_bsd_addr.c#L357
and
https://github.com/sctplab/usrsctp/blob/master/usrsctplib/netinet/sctp_bsd_addr.c#L405
are never freed.
On the other platforms we use a global list which is freed at the end. In the FreeBSD Kernel we use pointers to a global list of addresses which never goes away.
I need to check if we actually use these pointers in the userland stack, or if a NULL pointer can be provided. If we use the global list, it should be updated when addresses are added to deleted to the stack. Not sure if this happens. I need to check that.

@tuexen
Copy link
Member

tuexen commented Feb 20, 2016

OK. This should be fixed now. Can you retest?

@jswirl
Copy link
Author

jswirl commented Feb 22, 2016

usrsctptest6

@tuexen: Yes, it appears to be no longer leaking. Thanks so much for your help!

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

No branches or pull requests

3 participants