How to implement RAII / Drop for Sockets?

[DrTobe]

I have been working on a modem driver for a Quectel BG77 which is a modem that is controlled via AT commands over a UART. The network stack is implemented on the modem and the modem supports multiple sockets.

To me, it seems obvious to keep track of acquired sockets in a list and recycling/returning those as soon as TcpClientStack::close is called. Unfortunately, it seems to be impossible to guarantee that the close-method gets called at all. It is easy to imagine cases which will cause socket exhaustion in the long run:

let mut socket = tcp_stack.socket()?;
function_call_that_can_fail()?; // e.g. socket.send(...), function returns here, socket is leaked
tcp_stack.close(socket);

Normally, this would be tackled with RAII, i.e. implementing the Drop trait for the socket. But as far as I see, this is impossible (at least with the current trait definitions) because we need the &mut TcpStack to close the socket.

In the beginning, I thought that this could be solved with interior mutability. But to implement that, the socket type needs to contain a &RefCell<TcpClientStack> (which still is a reference to the TcpClientStack). First, I could not get this to compile at all. The closest I could get was type TcpSocket<'a> = Bg77TcpSocket<'a, ...>; which fails with "generic associated types are unstable". Secondly, regarding the lifetimes, the socket would borrow the tcp stack in TcpClientStack::socket(&mut self). So the tcp stack would be borrowed until the socket is destroyed which limits us to a single usable socket.

Personally, I would really like to implement this feature because not doing so brings us back to C-style manual resource management with all its downsides (even worse, because the ?-operator is harder to spot than a return).

The solution will most probably be based on interior mutability because the (possibly multiple) sockets require (possibly mutable) access to the underlying hardware. I have seen discussions here that want to save the driver developers from re-inventing this interior mutability wheel over and over again, so it would be nice to have a solution which is generic over different driver implementations I guess. So what are your thoughts and ideas on this?

[ryan-summers]

I've hit this myself a few times and I agree that it's easy to get wrong. Dropping sockets is definitely not something we want to do if they're being tracked internally by the stack structure. Perhaps #53 could help to solve this, but that API implies some internal &RefCell<Stack> as well I believe

[chrysn]

Some data point that may be helpful (even though not fully thought through as my own implementation is incomplete): The stack as I use it in riot-wrappers comes with sockets that are allocated inside the stack (which need to be pinned as that's what the backend requires).

Dropping a socket is as fine as it gets (well it leaks resources, as dropping often does), but if the stack is ever deallocated (which I don't fully support yet) and not all of its sockets were closed properly before that, dropping the stack (which is guaranteed to run through its pinning) panics.

For the example you give, a try! block, or a helper function (which may or may not make sense as a provided method of stacks) could catch this:

tcp_stack.with_socket(|s| {
    function_call_that_can_fail()?;
})?;

Such a with_socket would always close the socket at the end, having only handed out a &mut socket. I don't think it should be the general pattern, because where sockets are longer lived, ownership is desired, but comes at the cost of having to dispose of them properly.

[DrTobe]

Thanks for your suggestion but I do not think this resolves my problem because the responsibility of not leaking the resource is still on the caller/user side and that is exactly what I try to avoid.

I think I just found a way of achieving my goal, based on a RefCell obviously. I hope I can post this here in a few hours.

[DrTobe]

It works. The trick was to do/implement everything in a Bg77Driver and let the Bg77ClientStack and Bg77SocketHandle both take &RefCell<Bg77Driver>s. Like this, the TcpClientStack trait can be used as is and Drop can be implemented for the socket handles because they can get direct access to the underlying driver via the RefCell.

The following gist shows how all this can be set up:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=abdd8db35d70ca1dd37fb3d21d4a3445

Some observations on this:

1. Does this re-invent the SharableStack / shared-bus interior mutability wheel? Unfortunately, yes.
2. Because it does so, the Bg77ClientStack can be Copy (stack sharing is no problem at all).
3. The Drop implementation shows that the sockets do not actually need the &mut TcpClientStack at all to operate on the modem. So this implementation would allow a socket-directed API / trait as proposed in [Proposal] Socket-directed API #53.