Implementing "resource deallocation must succeed"

This is a followup to Should I use a linked list?

I have implemented an API that looks like this:

var port = try Port.init(allocator);
defer port.deinit();
while (true) {
    try port.sendTransactions();
    doApplication();
    try port.recvTransactions();
    std.thread.sleep(100);
}

Where port.recvTransactions() deallocates resources and port.sendTransactions() allocates resources.

However, I have done this incorrectly, since port.recvTransactions() may return an error, it is harder for the user to correctly manage the resources. I have violated resource deallocation must succeed.

Here are some addtional details:

  1. port.sendTransactions() sends data on an ethernet port. This send() may fail (for example, the cable is unplugged) and thus this function may return an error. This function also appends to a pre-allocated queue without possibilty of errors. The queue tracks what transactions are pending. Failing to release transactions is bad.
  2. Since failing to release a transaction is bad (basically a memory leak), port.deinit() asserts that there are no transactions in the queue.
  3. port.recvTransactions() calls recv on the ethernet port. This recv() may fail (for example, the cable is unplugged) and this this function may return an error. The function also releases all the pending transactions, regardless of whether they have been received.

Constraints:

  1. I do not want to expose the transaction queue to the user, the queue is fully allocated to a specific size at Port.init based on complex math that ensures the queue will never overflow. (tiger style)

Ideas:

  1. Change sendTransactions() to first release all transactions before sending. Inside sendTransactions we would errdefer releaseAllTransactions. Then recvTransactions() does not manage the transaction queue. In this API, it would not be user error to call recvTransactions() multiple times.

There is a problem within idea however:

var port = try Port.init(allocator);
defer port.deinit();
while (true) {
    try port.sendTransactions();
    doUserApplication() catch return error.UserError;
    port.recvTransactions();
    std.thread.sleep(100);
}

Now when the user decides to return an error, the port.deinit() assert will trigger since sendTransactions succeeded. There is no way to releaseTransactions.

I value the assert in port.deinit() because it helps me check my code, so I don’t want to just remove it…

How do you think I should change my API so that I can adhere to resource deallocation must succeed?

The first big question for me is, why do you want to associate recvTransactions with freeing resources in the first place? The name recvTransactions doesn’t tell that it’s intended to free resources from sendTransactions. The fact that you also release resources in receive, seems more of an implementation detail, than something you should enforce or encode in the API contract.

The function also releases all the pending transactions, regardless of whether they have been received.

This looks like an odd choice to me. You cannot control when a user calls recvTransactions so why does it even keep all pending transactions around in the first place, when they could get cleared at an arbitrary point in time anyways?

3 Likes

The user is interacting with a real-time system with deterministic latency. The user sends transactions which are executed and returned by remote hosts.

The user often desires to take advantage of the cpu cycles available in the time between send and recv. So the two steps are separated to allow the user to do stuff when the operations are pending.

If the user calls recv at the correct time (some minimum time from send) they can expect to get everything. If some of the transactions did not return, the transactions can be assumed to be lost (cable broken, corrupted ethernet frames, etc).

I made a mistake in the original post, its actually most likely to be used like this


var port = try Port.init(allocator);
defer port.deinit();
while (true) {
    recv();
    doApplication();
    send();
    sleepUntilNextCycle();

}

I think I will need to just get rid of this whole notion of resources and instead have send() mark all transactions as sent.

And change recv() to being allowed to be called multiple times.

I may be oversimplifying things in my head but if this recv API is all you want the user to have in regards to resource deallocation what would be the issue of just having a defer dealloc within recv? This would guarantee on error or success the deallocation happens no?

I’m a bit tired so sorry if I’m missing something obvious here.