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.

Sign up for GitHub

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

Jump to bottom

unix-fd listener should not remove sockets on deregister() #364

Open
ensc opened this issue Jun 11, 2021 · 3 comments · May be fixed by #442
Open

unix-fd listener should not remove sockets on deregister() #364

ensc opened this issue Jun 11, 2021 · 3 comments · May be fixed by #442
Labels
bug Something isn't working

Comments

@ensc
Copy link

ensc commented Jun 11, 2021

actix-server runs

actix-net/actix-server/src/socket.rs

Lines 74 to 84 in 605ec25

fn deregister(&mut self, registry: &Registry) -> io::Result<()> {
match *self {
MioListener::Tcp(ref mut lst) => lst.deregister(registry),
#[cfg(unix)]
MioListener::Uds(ref mut lst) => {
let res = lst.deregister(registry);
// cleanup file path
if let Ok(addr) = lst.local_addr() {
if let Some(path) = addr.as_pathname() {
let _ = std::fs::remove_file(path);

This deregister function can be called as part of flow control as response to Pause command

actix-net/actix-server/src/accept.rs

Lines 286 to 292 in 605ec25

Some(WakerInterest::Pause) => {
drop(guard);
if !self.paused {
self.paused = true;
self.deregister_all(sockets);

When resuming operations, the socket file is gone and clients can not connect to the server anymore.

The cleanup should happen only when connection is closed finally (Stop command?)
@fakeshadow
Copy link
Contributor

#97

@ensc
Copy link
Author

ensc commented Jun 16, 2021

My use case for unix fds are CI tests: Instead of listening on tcp sockets and implementing ways to find free port numbers, I use unix sockets. Then, I can call the server like

curl --unix-socket /tmp/foo http://server.example.com

While doing tests, somehow the Pause event is triggered (I am not sure why; perhaps because I use SIGSTOP on the server between test steps), and things stop to work after

  1. curl --unix-socket /tmpfoo --> ok

  2. actix-net removes /tmp/foo

  3. curl --unix-socket /tmpfoo --> fails because file does not exist anymore

My workaround for now is to let actix-net listen on a temporary socket and rename it later to the real one.

@hommeabeil
Copy link

I got the same issue with a really simple server which listen on unix socket. Form it it when we go through the backpressure method, which deregister the socket. Here is a backtrace taken with gdb

#0  std::fs::remove_file (path=0x7ffff7787846) at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/std/src/fs.rs:1529
#1  0x00005555558b0e0d in <actix_server::socket::SocketListener as mio::event_imp::Evented>::deregister (self=0x7fffec000d80, poll=0x7ffff7788e30)
    at /usr/src/debug/sx-sts/0.1.0-r0/cargo_home/bitbake/actix-server-1.0.4/src/socket.rs:137
#2  0x00005555558fac89 in mio::poll::Poll::deregister (self=0x7ffff7788e30, handle=0x7fffec000d80)
    at /usr/src/debug/sx-sts/0.1.0-r0/cargo_home/bitbake/mio-0.6.23/src/poll.rs:910
#3  0x00005555558a80bd in actix_server::accept::Accept::backpressure (self=0x7ffff7788e30, on=true)
    at /usr/src/debug/sx-sts/0.1.0-r0/cargo_home/bitbake/actix-server-1.0.4/src/accept.rs:383
#4  0x00005555558a86f5 in actix_server::accept::Accept::accept_one (self=0x7ffff7788e30, msg=...)
    at /usr/src/debug/sx-sts/0.1.0-r0/cargo_home/bitbake/actix-server-1.0.4/src/accept.rs:437
#5  0x00005555558a979b in actix_server::accept::Accept::accept (self=0x7ffff7788e30, token=0)
    at /usr/src/debug/sx-sts/0.1.0-r0/cargo_home/bitbake/actix-server-1.0.4/src/accept.rs:475
#6  0x00005555558a62ca in actix_server::accept::Accept::poll (self=0x7ffff7788e30)
    at /usr/src/debug/sx-sts/0.1.0-r0/cargo_home/bitbake/actix-server-1.0.4/src/accept.rs:257
#7  0x00005555558a5071 in actix_server::accept::Accept::start::{{closure}} ()
    at /usr/src/debug/sx-sts/0.1.0-r0/cargo_home/bitbake/actix-server-1.0.4/src/accept.rs:170
#8  0x00005555558cb093 in std::sys_common::backtrace::__rust_begin_short_backtrace (f=...)
    at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/std/src/sys_common/backtrace.rs:125
#9  0x00005555558ddb95 in std::thread::Builder::spawn_unchecked::{{closure}}::{{closure}} ()
    at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/std/src/thread/mod.rs:474
#10 0x00005555558a9e85 in <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once (self=..., _args=())
    at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/std/src/panic.rs:322
#11 0x00005555558cdb33 in std::panicking::try::do_call (data=0x7ffff77894c8 "\000")
    at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/std/src/panicking.rs:381
#12 0x00005555558d8cad in __rust_try ()
#13 0x00005555558cc4d4 in std::panicking::try (f=...) at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/std/src/panicking.rs:345
#14 0x00005555558aa275 in std::panic::catch_unwind (f=...) at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/std/src/panic.rs:396
#15 0x00005555558dd95e in std::thread::Builder::spawn_unchecked::{{closure}} ()
    at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/std/src/thread/mod.rs:473
#16 0x000055555589a6fe in core::ops::function::FnOnce::call_once{{vtable-shim}} ()
    at /usr/src/debug/libstd-rs/1.49.0-r0/rustc-1.49.0-src/library/core/src/ops/function.rs:227
#17 0x0000555555dc728a in std::sys::unix::thread::Thread::new::thread_start ()
#18 0x00007ffff7edee24 in start_thread (arg=<optimized out>) at pthread_create.c:477
#19 0x00007ffff7cc9e9f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

What can we do about it ? Is simply removing the fs::remove_file should work ? My guess is we will be leaking the file in a clean exit ?

@robjtede robjtede added the bug Something isn't working label Oct 6, 2021
@hommeabeil hommeabeil linked a pull request Feb 3, 2022 that will close this issue
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

do not remove the uds socket on pause hommeabeil/actix-net
4 participants
@ensc @robjtede @hommeabeil @fakeshadow