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

TlsAcceptor::accept timeout and shutdown #90

Open
incker opened this issue Nov 12, 2024 · 5 comments
Open

TlsAcceptor::accept timeout and shutdown #90

incker opened this issue Nov 12, 2024 · 5 comments

Comments

@incker
Copy link

incker commented Nov 12, 2024

Hello!

I think that wrapping TlsAcceptor::accept in tokio::time::timeout can leads to memory leaks. But I am not sure.

Any way, it will be good to have some gracefull shutdown on timeout
Maybe there need a way to make TlsAcceptor::accept_with_timeout and in case of timeout acceptor will send shutdown().await

@djc
Copy link
Member

djc commented Nov 13, 2024

I think that wrapping TlsAcceptor::accept in tokio::time::timeout can leads to memory leaks. But I am not sure.

Why do you think so? If this was the case it seems likely that someone would have found it by now.

As for the accept with timeout, I think this is a duplicate of #4.

@incker
Copy link
Author

incker commented Nov 13, 2024

Unfortunately, I am not sure about. But looks like if not make shutdown().await explicitly, for me the memory usage grow day by day. I can not proof it. I can only tell that 80% probability that memory leaks were made by tokio-rustls. Maybe later I could try to reproduce it to know it for sure.

TlsAcceptor::accept with tokio::time::timeout is only one variant where I can not do shutdown() explicitly.
And yes, making timeout like here will be super #4

@quininer
Copy link
Member

Unless you're using some io_uring-based TCP stream, and they're going to leak memory on drop for safety. otherwise, the drop accept future action itself won't cause any memory leak.

If you do use a stream that causes a leak on drop, then call shutdown won't help either. It's best to have stream call the cancel API on drop to ensure that buffer can be reclaimed.

@incker
Copy link
Author

incker commented Nov 30, 2024

Unless you're using some io_uring-based TCP stream

How ti know if I use io_uring-based TCP stream?
I use tokio. And as far as I know it is not using io_uring (maybe epoll) I use linux

I have looked tokio-rustls dependencies, And I have not also found io-uring
And in my Cargo.lock there no io-uring

Maybe it is some type of C library. In case yes, what is the alternative, and how can I switch to it at least for tests

Forward thank you

@quininer
Copy link
Member

quininer commented Dec 1, 2024

from the description, I don't think it's related to io-uring, and it would be helpful if you could provide a minimal reproducible example.

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