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

Windows mio leaks memory when poll does not retrieve any event #1146

Closed
dtacalau opened this issue Nov 12, 2019 · 8 comments
Closed

Windows mio leaks memory when poll does not retrieve any event #1146

dtacalau opened this issue Nov 12, 2019 · 8 comments
Assignees
Labels
windows Related to the Windows OS.
Milestone

Comments

@dtacalau
Copy link
Contributor

Steps to reproduce:

  1. Simplify test smoke_test_tcp_listener from tcp_listener.rs, to just register a listener and do poll without connecting any stream:
fn smoke_test_tcp_listener<F>(addr: SocketAddr, make_listener: F)
where
    F: FnOnce(SocketAddr) -> io::Result<TcpListener>,
{
    let (mut poll, mut events) = init_with_poll();

    let listener = make_listener(addr).unwrap();
    let address = listener.local_addr().unwrap();

    poll.registry()
        .register(&listener, ID1, Interests::READABLE)
        .expect("unable to register TCP listener");

    poll.poll(&mut events, Some(std::time::Duration::from_millis(500))).expect("unable to poll");
  1. Run this test under drmemory Windows profiler and leak detector.
  2. Profiler reports a memory leak:
Error #5: LEAK 136 direct bytes 0x00000091448cfb30-0x00000091448cfbb8 + 56 indirect bytes
# 0 replace_RtlAllocateHeap                                                    [d:\drmemory_package\common\alloc_replace.c:3771]
# 1 alloc::alloc::alloc                                                        [/rustc/625451e376bb2e5283fc4741caa0a3e8a2ca4d54\src\liballoc\alloc.rs:84]
# 2 alloc::alloc::exchange_malloc                                              [/rustc/625451e376bb2e5283fc4741caa0a3e8a2ca4d54\src\liballoc\alloc.rs:206]
# 3 alloc::sync::Arc<>::new<>                                                  [/rustc/625451e376bb2e5283fc4741caa0a3e8a2ca4d54\src\liballoc\sync.rs:306]
# 4 mio::sys::windows::selector::SelectorInner::_alloc_sock_for_rawsocket      [C:\work\git\mio\src\sys\windows\selector.rs:676]
# 5 mio::sys::windows::selector::SelectorInner::register<>                     [C:\work\git\mio\src\sys\windows\selector.rs:533]
# 6 mio::sys::windows::selector::Selector::register<>                          [C:\work\git\mio\src\sys\windows\selector.rs:400]
# 7 mio::sys::windows::tcp::{{impl}}::register                                 [C:\work\git\mio\src\sys\windows\tcp.rs:451]
# 8 mio::net::tcp::listener::{{impl}}::register                                [C:\work\git\mio\src\net\tcp\listener.rs:142]
# 9 core::sync::atomic::AtomicUsize::load                                      [/rustc/625451e376bb2e5283fc4741caa0a3e8a2ca4d54\src\libcore\sync\atomic.rs:1296]
#10 mio::poll::Registry::register<>                                            [C:\work\git\mio\src\poll.rs:494]
#11 hello_world::smoke_test_tcp_listener<> core::result::Result<>>             [C:\work\rust-learning\src\main.rs:127]

===========================================================================
FINAL SUMMARY:

DUPLICATE ERROR COUNTS:

SUPPRESSIONS USED:

ERRORS FOUND:
      0 unique,     0 total unaddressable access(es)
      4 unique,     4 total uninitialized access(es)
      0 unique,     0 total invalid heap argument(s)
      0 unique,     0 total GDI usage error(s)
      0 unique,     0 total handle leak(s)
      0 unique,     0 total warning(s)
      1 unique,     1 total,    192 byte(s) of leak(s)
      0 unique,     0 total,      0 byte(s) of possible leak(s)
ERRORS IGNORED:
     11 potential error(s) (suspected false positives)
         (details: C:\Users\dan\AppData\Roaming\Dr. Memory\DrMemory-hello-world.exe.6532.001\potential_errors.txt)
     14 unique,    14 total,   2517 byte(s) of still-reachable allocation(s)
         (re-run with "-show_reachable" for details)
Details: C:\Users\dan\AppData\Roaming\Dr. Memory\DrMemory-hello-world.exe.6532.001\results.txt
dtacalau added a commit to dtacalau/mio that referenced this issue Nov 12, 2019
…any event. In case there are no events retrieved by poll, SockState is leaked because of one of its Arc wrapped in self_wrapped. To prevent this, make sure to decrease reference count of the SockState Arc wrapped by the self_wrapped in case the overlapped is not used.

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
@Thomasdezeeuw Thomasdezeeuw added the windows Related to the Windows OS. label Nov 13, 2019
@Thomasdezeeuw Thomasdezeeuw added this to the v0.7 milestone Nov 13, 2019
dtacalau added a commit to dtacalau/mio that referenced this issue Nov 15, 2019
…any event. Pending sockets should be cancelled on inner selector drop, otherwise SockState is leaked. SockState Arc, wrapped in self_wrapped, is used as overlapped data

for polling through Afd NtDeviceIoControlFile, so the SockState memory cannot be dropped unless this operation complete successfuly or is cancelled.

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
dtacalau added a commit to dtacalau/mio that referenced this issue Nov 21, 2019
… improvements. All registered sock_states are saved in a slab data structure. Sock_states are removed from slab during deregister. A reregister expects to find the sock_state saved in slab already. During Selector drop all sock_states from the slab are cancelled and dropped. The slab saves sock_states as pairs of (key, sock_state). The key is also saved in the sock_state and used as overlapped data. Later, when receiving the completion event, the key is used to retrieve the sock_state. The slab will never get full of unusable sock_states which have been marked for deletion because, once a sock_state has been marked for deletion it will also be removed from the slab. Specific cases which should trigger the removal are: 1. a sock_state for which completion returned error, i.e ERROR_INVALID_HANDLE; 2. a sock_state for which a an LOCAL_CLOSE event was retrieved;

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
dtacalau added a commit to dtacalau/mio that referenced this issue Nov 23, 2019
…k state Arc is blocked by a pending afd poll request, this reference will leak if there's no explicit poll done before the program terminates. That's why

doing an explicit "select" during iner selector drop ensures all sock state references are freed.

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
@dtacalau
Copy link
Contributor Author

@Thomasdezeeuw please assign this issue to me, thanks.

dtacalau added a commit to dtacalau/mio that referenced this issue Nov 25, 2019
…keep things simple in the Drop impl.

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
piscisaureus pushed a commit to piscisaureus/mio that referenced this issue Nov 26, 2019
Fixed leaked sock states, issue tokio-rs#1146. A reference of the sock state
Arc is blocked by a pending AFD poll request, this reference will leak
if there's no explicit poll done before the program terminates. That's
why doing an explicit "select" during iner selector drop ensures all 
sock state references are freed.

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
piscisaureus pushed a commit to piscisaureus/mio that referenced this issue Nov 26, 2019
A reference of the sock state Arc is blocked by a pending AFD poll
request, this reference will leak if there's no explicit poll done
before the program terminates. That's why doing an explicit "select"
during inner selector drop ensures all sock state references are freed.

Fixes: tokio-rs#1146

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
dtacalau added a commit to dtacalau/mio that referenced this issue Nov 27, 2019
…k state Arc is blocked by a pending afd poll request, this reference will leak if there's no explicit poll done before the program terminates. That's why

doing an explicit "select" during iner selector drop ensures all sock state references are freed.

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
dtacalau added a commit to dtacalau/mio that referenced this issue Nov 27, 2019
…keep things simple in the Drop impl.

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
@Thomasdezeeuw Thomasdezeeuw reopened this Nov 28, 2019
@Thomasdezeeuw
Copy link
Collaborator

Commit 2b68a4b fixed this, but I'm not too happy with the solution.

dtacalau added a commit to dtacalau/mio that referenced this issue Dec 4, 2019
A reference of the sock state Arc is blocked by a pending AFD poll
request, this reference will leak if there's no explicit poll done
before the program terminates. That's why doing an explicit "select"
during inner selector drop ensures all sock state references are freed.

Fixes: tokio-rs#1146

Signed-off-by: Daniel Tacalau <dst4096@gmail.com>
@Thomasdezeeuw Thomasdezeeuw modified the milestones: v0.7, v1.0 Dec 4, 2019
@JRAndreassen
Copy link

Hi...

I have an odd one and wondering if it is related...

I have a case where there is a runaway process in a futures::block_on.
I've had it happen inside both reqwest and in trust-dns crates.

It seems to be happening when there is a NIC reset, which happens on logins (for some silly reason)...

Just asking for a sanity check
Thanks
JR

tokio:0.2.22 / mio v0.6.22
stable-x86_64-pc-windows-msvc - Up to date : 1.47.0 (18bf6b4f0 2020-10-07)
stable-x86_64-unknown-linux-gnu - Up to date : 1.47.0 (18bf6b4f0 2020-10-07)
nightly-x86_64-pc-windows-msvc - Update available : 1.49.0-nightly (3525087ad 2020-10-08) -> 1.49.0-nightly (07e968b64 2020-10-27)
nightly-x86_64-unknown-linux-gnu - Update available : 1.49.0-nightly (3525087ad 2020-10-08) -> 1.49.0-nightly (07e968b64 2020-10-27)

@Thomasdezeeuw
Copy link
Collaborator

Can you try Mio 0.7? It should be fixed it that.

@JRAndreassen
Copy link

@Thomasdezeeuw ,
Thanks for answering...
Unfortunately, my dependency chain does not support that...
I'm looking at alternatives.

@Thomasdezeeuw
Copy link
Collaborator

Any chance you can update to Tokio v0.3? The Windows implementation was rewritten completely, so honestly I don't want to spend too much time on fixing this.

@JRAndreassen
Copy link

JRAndreassen commented Oct 28, 2020

@Thomasdezeeuw 👍
I understand...
I'll work around until the dependency chain catches up...
First step is to get rid of futures::block_on under tokio...
We'll see where it goes from there.

Thanks for lookin into it.
JR

@Thomasdezeeuw
Copy link
Collaborator

This has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
windows Related to the Windows OS.
Projects
None yet
Development

No branches or pull requests

3 participants