A memory leak in Windows #919
Comments
|
My investigation so far: Windows side of UdpSocket uses overlapped memory to communicate with IOCP. This seem to include Tcp side documentation: So the implementation should match every I added a trace debug for Since the refcount is still over1, the inners of Also it looks like UDP's |
|
This might be related to #776. |
This adds support for performing non-blocking network operations, such
as reading and writing to/from a socket. The runtime API exposed is
similar to Erlang, allowing one to write code that uses non-blocking
APIs without having to resort to using callbacks. For example, in a
typicall callback based language you may write the following to read
from a socket:
socket.create do (socket) {
socket.read do (data) {
}
}
In Inko, you instead would (more or less) write the following:
import std::net::socket::TcpStream
let socket = try! TcpStream.new(ip: '192.0.2.0', port: 80)
let message = try! socket.read_string(size: 4)
The VM then takes care of using the appropriate non-blocking operations,
and will reschedule processes whenever necessary.
This functionality is exposed through the following runtime modules:
* std::net::ip: used for parsing IPv4 and IPv6 addresses.
* std::net::socket: used for TCP and UDP sockets.
* std::net::unix: used for Unix domain sockets.
The VM uses the system's native polling mechanism to determine when a
file descriptor is available for a read or write. On Linux we use epoll,
while using kqueue for the various BSDs and Mac OS. For Windows we use
wepoll (https://github.com/piscisaureus/wepoll). Wepoll exposes an API
that is compatible with the epoll API, but uses Windows IO completion
ports under the hoods.
When a process attempts to perform a non-blocking operation, the process
is registered (combined with the file descriptor to poll) in a global
poller and suspended. When the file descriptor becomes available for a
read or write, the corresponding process is rescheduled. The polling
mechanism is set up in such a way that a process can not be rescheduled
multiple times at once.
We do not use MIO (https://github.com/tokio-rs/mio), instead we use
epoll, kqueue, and wepoll (using
https://crates.io/crates/wepoll-binding) directly. At the time of
writing, while MIO offers some form of support for Windows it comes with
various issues:
1. tokio-rs/mio#921
2. tokio-rs/mio#919
3. tokio-rs/mio#776
4. tokio-rs/mio#913
It's not clear when these issues would be addressed, as the maintainers
of MIO appear to not have the experience and resources to resolve them
themselves. MIO is part of the Google Summer of Code 2019, with the goal
of improving Windows support. Unfortunately, this likely won't be done
before the end of 2019, and we don't want to wait that long.
Another issue with MIO is its implementation. Internally, MIO uses
various forms of synchronisation which can make it expensive to use a
single poller across multiple threads; it certainly is not a zero-cost
library. It also offers more than we need, such as being able to poll
arbitrary objects.
We are not the first to run into these issues. For example, the Amethyst
video game engine also ran into issues with MIO as detailed in
https://community.amethyst.rs/t/sorting-through-the-mio-mess/561.
With all of this in mind, I decided it was not worth the time to wait
for MIO to get fixed, and to instead spend time directly using epoll,
kqueue, and wepoll. This gives us total control over the code, and
allows us to implement what we need in the way we need it. Most
important of all: it works on Linux, BSD, Mac, and Windows.
|
@PerfectLaugh or @oherrala can either of you conform the memory leak doesn't exists on current master? I don't this is does any more as #776 is resolved. |
|
I assume it's this PR: #1034 Adding it here just for cross referencing. |
|
Thanks @PerfectLaugh for working on this! I quickly tested the current master (e134e79) in Windows 10 using Dr. Memory 2.2.0 and my previous test code (adjusted for new mio) and Dr. Memory seems to be happy. No more leak like what was found above. My program: use std::io;
use std::net::{Ipv4Addr, SocketAddr};
use std::thread::sleep;
use std::time::Duration;
use env_logger;
use mio::net::UdpSocket;
use mio::{Interests, Poll, Token};
fn main() -> io::Result<()> {
env_logger::init();
let target = SocketAddr::new(Ipv4Addr::new(192, 168, 1, 1).into(), 53);
let buf = &[0x00; 1400];
let socket = UdpSocket::bind(SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), 0))?;
let poll = Poll::new()?;
poll.registry()
.register(&socket, Token(0), Interests::READABLE)
.unwrap();
socket.send_to(buf, target)?;
sleep(Duration::from_secs(1));
Ok(())
} |
|
@Thomasdezeeuw @PerfectLaugh is there any change to get this backported into mio 0.6? Trust-DNS on Windows is affected on this issue as well as probably other users of Tokio in Windows. Without fix in mio 0.6 I'm afraid it might take a while to get the ecosystem updated and fix deployed. Issues hopefully solved by the fix:
|
|
It could be possible to backport, but I would like the code to mature some on 0.7 first. There could be other bugs introduced in the change given that it is a rewrite. The more testing we get on 0.7, the sooner we can backport :-) |
|
Closing this for v0.7 and moving it to a new maybe backport to v0.6 milestone. |
I have made a backport. But due to 0.7 only supports edge-triggered mode. Level-triggered in the backport will cause error Here is the code: https://github.com/PerfectLaugh/mio-compat |
|
Congratulation on 0.7.0 release! I tested the release with following program: [package]
name = "miotest"
version = "0.1.0"
authors = ["Ossi Herrala <[email protected]>"]
edition = "2018"
[dependencies]
env_logger = "0.7"
mio = { version = "0.7", features = [ "os-poll", "udp" ] }use std::io;
use std::net::{Ipv4Addr, SocketAddr};
use std::thread::sleep;
use std::time::Duration;
use env_logger;
use mio::{Interest, Poll, Token};
use mio::net::UdpSocket;
fn main() -> io::Result<()> {
env_logger::init();
let target = SocketAddr::new(Ipv4Addr::new(192, 168, 1, 1).into(), 53);
let buf = &[0x00; 1400];
let mut socket = UdpSocket::bind(SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), 0))?;
let poll = Poll::new()?;
poll.registry()
.register(&mut socket, Token(0), Interest::READABLE)
.unwrap();
socket.send_to(buf, target)?;
sleep(Duration::from_secs(1));
Ok(())
}And Dr. Memory 2.3.0 reported no issues. I'll close this issue and hope that upstreams pick up 0.7 instead of requiring a backport fix to 0.6. |
The following code can be used to trigger a memory leak in Windows. I first found this with
trust-dns-client(see https://github.com/bluejekyll/trust-dns/issues/692), but the issue can be isolated intomioonly.Trace level debug output of a run:
And Dr. Memory results of the leak:
The text was updated successfully, but these errors were encountered: