TestSockets are not closed in many tests

[gpotter2]

I've been investigating issues failures on PyPy and it appears it fails because we open too many pipes during the tests:

Why PyPy and not CPython you may ask? Because PyPy has a different gc (see this) that doesn't call __del__ as early as CPython (as soon as the socket is unreferenced), so sockets pile up.

It's easy to find tons of unclosed sockets in the tests (grep -R TestSocket then a little bit of looking around) https://github.com/secdev/scapy/blob/9420c2229bf5330c2cc580f114f63f920a68db10/test/contrib/isotpscan.uts#L72 https://github.com/secdev/scapy/blob/9420c2229bf5330c2cc580f114f63f920a68db10/test/contrib/isotpscan.uts#L163-L164

So we should probably perform some cleanups on those. Feel free to contribute !

[polybassa]

Thanks a lot for this investigation.. I'll take care of it, since I caused these issues.

What do you recommend? Using with Testsocket() as sock: all over the place?

[gpotter2]

You could do that or just call .close(), ideally in a try finally but as those are tests it's fine if you don't. (we would have a failure anyways)

It's alright those happen a lot. At some point automatons broke down entirely because of a similar issue.. they're pretty hard to troubleshoot

[polybassa]

found a easy solution.. hope it holds.

[gpotter2]

Okay that was clearly not enough. Tests on PyPy2 are still crashing after thousands of TestSocket are hold in memory. There is a bug somewhere in an automotive test that keeps creating some and doesn't close them

For instance: https://github.com/secdev/scapy/runs/7296916202?check_suite_focus=true

[polybassa]

Thanks.. I'll have a look!

[polybassa]

This log looked like a race-condition was triggered... I've added some debug code to fail fast, once this error happens again.

[gpotter2]

I do not believe that the issue here is that it fails to close the sockets, the real issue is that there are thousand and thousands of sockets to close in one go in the first place. You can't rely on __del__ on PyPy to clear the sockets for you, because of limitations, so you're supposed to have closed them manually beforehand.

[polybassa]

In the current version on master, I'm already calling close manually on all opened sockets after each testcase: https://github.com/secdev/scapy/blob/2c92b0350ab5df8ea0adc164fb4441c979bec568/test/contrib/automotive/scanner/uds_scanner.uts#L82

This should ensure, that all sockets get closed, but maybe I'm missing something.

[polybassa]

The current CI results, clearly show, that OSX and Windows CI works as expected. Only on Linux based systems, somehow the objects doesn't get destroyed.

[gpotter2]

Looks much more stable ! Thanks a lot polybassa

[polybassa]

Finally :-D

[polybassa]

I'm still seeing a little instability in the GMLAN Scanner tests. To let you know that I have an eye on that ones.