Modernize: Sink Functions

One of the guidelines from Modern C++ is to avoid using raw new and delete. Instead, you should use a smart pointer, a container or other RAII object. Today I’d like to focus on so-called ‘sink functions’ that takes ownership of input parameters. How can we modernize code around such calls?

Intro  

Briefly: a sink function is a function that takes ownership of an input pointer. Such function is now responsible for the allocated memory/resource. It can pass the ownership further or manage it on its own.

Here are links for more definitions: Cpp Wiki: Move Constructor, Herb Sutter: Smart pointer parameters,Simplify C++: Move Semantics

Here’s a little diagram:

As it’s shown on the picture: Foo() creates a resource - ptr - and passes it to Bar() then it’s transferred to DoStuff() that (hopefully) finalizes it and destroys.

In legacy code, you could probably see code similar to:

void Foo() {
  MyType* pMyObject = new MyType(); // << !

  // change pMyObject somehow...

  HandleMyType(pMyObject); // transfer ownership
}

// transfer ownership of pMyObject 
void HandleMyType(MyType* pMyObject) {
  // handle the pointer...

  delete pMyObject; // finalize...
}

Foo() - is a source type function.
HandleMyType() - is a sink type function.

To be more specific we’re talking here about parameters that are usually movable only and not copyable. Like pointers. If you pass a value type like int param then there’s no need to pass ownership.

Also, usually we’re talking about allocated memory, but it can be any kind of resource: like a file handle, network connection, DB connection, some unique state, etc.

The above code is as usually very simple. In a real example, the ownership can be passed multiple levels down in the call stack hierarchy. The pointer might be even stored in some object that lives much longer than the ‘creation’ point. We could invent lots of examples here. See the picture below that shows many layers where the resource might be processed and eventually released.

Ok… you’ve seen code like that, but is it safe and modern? Definitely not!

The problem  

In the above code, you can clearly see a ‘contract’ between one function and another: who is the owner of the resource/pointer.

Can you violate this contract?

Yes… and it’s very simple!

You can easily forget to delete allocated memory, forget about the ownership.

While it’s relatively easy to spot such problem in a short example like the above, it might be a pain in real code! Probably you know what I mean here. There’s a high chance you’ve already tracked bugs/leaks like that :)

The problem is that the contract is almost “verbal” only, or “comment” only. The compiler cannot help you here.

So what if someone doesn’t free the memory?

What if the foo() method needs to exit early (because of some error)?

Modernize  

To fix all of the above problems we need to use one powerful mechanism: RAII. Particularly in the form of unique_pointer.

Use unique_ptr  

How about the improved version:

void FooU() {
  auto pMyObject = make_unique<MyType>(); // <<

  // change pMyObject somehow...

  HandleMyTypeU(move(pMyObject));
}

void HandleMyTypeU(unique_ptr<MyType> pMyObject) {
  // handle the pointer...
}

A bit better?

Can you violate the contract now?

It’s quite hard!

Do you get help from the compiler?

Yes! It will report compile time errors!

Can you leak the memory?

Not easily!

Is the code more expressive and clean?

Yes!

So many benefits by just using a simple pointer wrapper! Also from the performance point of view, you don’t lose anything, because unique_ptr is just a tiny wrapper around a raw pointer and has the same size as the raw pointer.

Since unique_ptr is a movable-only type (not copyable), you’ll get a compile-timer error if you just want to copy the pointer. That way you need to move the pointer explicitly and that way pass the ownership.

BTW: small improvement: there’s auto used in the example. That way we don’t need to write:

unique_ptr<MyType> pPtr = make_unique<MyType>();

And it follows “AAA” rule.

Partial solution  

There’s also a partial solution to our original problem. Sometimes you cannot change the sink function - this might happen when you’re using some third party library. What you can do is at least to be safer at ‘your’ side of the code.

Basically, I would still create an unique_ptr, but since you cannot pass it to the old sink function, you need to release it and pass as a raw pointer.

void FooUP() {
  auto pMyObject = std::make_unique<MyType>(); // <<

  // change pMyObject somehow...

  if (condition) // the pointer will be deleted automatically here!
    return;

  HandleMyType(pMyObject.release());
}

As you see the code uses the release() method to remove ownership from the pointer. It also returns the raw pointer so we can use it.

What you get here?

Possibly not that much, but at least when your function needs to return early (before passing the pointer), you can be sure the memory won’t leak.

Also, please bear in mind that in real code that ownership can be passed in multiple levels of call stack:

void Foo() {
  // ...
  FooInner(ptr);
}

void FooInner() {
  // ...
  FooX_Inner(ptr);
}

void FooX_Inner() {
  // ...
  FooLib_Inner(ptr); // cannot use unique_ptr here!
 }

// ...

Let’s assume that FooLib_Inner cannot use unique_ptr. Still, I believe, there’s a sense in modernizing Foo and FooX_Inner. The code will be safer, and maybe at some point, you’ll be able to improve library code as well. Maybe the library will update, and it will support unique_ptr at some point.

Summary  

Play with the code here: @coliru

This was a quick and straightforward post. I believe that by reducing a number of raw new and delete you can end up with much safer code. One way of doing this is to use unique_ptr when passing to sink type functions.

I hope it helps.

What are your strategies for limiting usage of new/delete?

Share your experience in comments below the article.