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?

2 Likes