On the Topic of "Smart Pointers" and the Factory Create Pattern

This is a topic I have visited a few times over the years:

2016, 2016 on SDL2, 2019, 2023 on control block, and I visited it professionally on the now cancelled AAA MMO Game Engine with Zenimax Online; the specific pattern within which came from my original 2016 post, just with a few tweaks and expansions over the years.

Its just one of those things which comes up time and again in the C++ space, much like my crusade against return early in performance code... cough.

And it came up again just this week, where given a coding test I immediately did not like seeing raw pointers being used, so quickly refactored it to use a factory create pattern, except I was in a rush and didn't have the luxury of a whole game engine backing me up.

So denuded of even my own post I quickly used a different approach, so use a hidden (private) member struct in the constructor.  The constructor remains public so std::make_shared or std::make_unique can access them without hinderance from the static create factory member function, but that a regular user can not use a flat "operator new" as they can not access the private struct.... Sounds complex, but it's really not, and you will even find this exact pattern on CPP Reference sites across the web, let us quickly output an example (and I'll assume you've taken a look at my prior post from 2016 above for the actual "shared pointer friendship" I would prefer).

struct Thing

{

private:

   struct HiddenInternal {};    // Just an empty struct

public:
    using Ptr = std::shared_ptr<Thing>;

    static Ptr Create()
    {
        return std::make_shared<Thing>(HiddenInternal{});
    }

    explicit Thing([[maybe_unused]] HiddenInternal internal)

   {

   }   

};

And so with this code we can only get a Ptr instance here from the static Create call.

Now I've set the scene, what am I asking myself?  I'm asking what is the compile time and what is the runtime effect of these two patterns?

Compiletime my suspicions are that the shared_pointer friendship stuff in my original post is more weight to carry, there's a macro in my more complex implementations, so that's pre-processor overhead, there's expansion of the code at compile time.

But there's also this HiddenInternal in this new, simpler, implementation based solve; I believe there will even be compiler ellision of the HiddenInternal as its empty and does nothing; I just need to observe and understand this topic better.

Lets start with Compiler Explorer and my first confirmative discovery, just as I suspected, the compiler is very smart smart and it will easily see a trivial class such as mine above, or one with a single member, and fold it away to nothing in compiler ellision.

And this is a great thing!  It proves at zero runtime overhead whilst we have code fully communicative of the intent, self documenting code is a wonderful thing, and this is an interesting and eye catching pattern in the code.... Such that I believe even a new set of eyes meeting it would appreciate there is something special intended and they would take care to use the class properly, avoiding memory leaks for us along the way.

Good code health is as important as the final result, I especially take pride in code which can run a long duration.  I've always worked on long lived project timelines.

So HiddenInternal can result in no overhead when the class is trivial, what about when the class is complex?

The first thing I notice is that the factory creation function itself generates no code, yet we benefit from its intent at compile time and in the usage.  There is of course the control block creation for the shared pointer in this instance and I hold that in my mind, but the cost to pay back in intent of the code far outweighs the control block.

The constructor itself is interesting.... 

Removing the HiddenInternal gives no difference, so the compile has still removed the empty class, but enforces it's use in the code... This is a total win-win... The cost at compile time, one compile time ellision in one translation unit which is neglegable.

The runtime overhead zero.

What about the preprocessor and friendship.... The friend keyword itself is zero cost, the wrapping of it in a macro is therefore arguably a different conversation to have.  Resolving a friendship however is not so cost effective, the friend keyword adds a symbol, typically with a cost of O(1).

The look up from any other resolve is therefore either going to be O(1) or O(log N) where N is the number of other appearances of the name, this look up is going to be the compile time cost.  In practical terms this is going to be a very small duration, perhaps even below measurable in the pantheon that is a build across multiple cores.

A friendship link itself however costs nothing in code generation time, so we do not hold up the compile per se, we do however add a cost in the symbol table build.

Once something is in the symbol table it will result in complexity when refactoring code, potentially more rebuilds if the friendships, or classes within change.

In conclusion, if I have the option to set up a friendship to the underlying control block (see 2016) I would, but equally to keep the local complexity low I would also consider the Hidden private member class as a "trick".

C++: Undefined behaviour from realloc and the Clang Optimizer

I was wondering around in some system code and found a strange behaviour between two modes in a piece of code, where the code switches from double to triple buffer mode there's a problem, we've already got two buffers of everything we just want to allocate another but the underlying structure for sets of buffers wants to own them all... So the code went from:

SetOfBuffers

{

Buffer* one;

Buffer* two;

}

To:

SetOfBuffers

{

Buffer* one;

Buffer* two;

Buffer* three;

}

Creating the third buffer is fine:

SetOfBuffers::three = malloc(X);

But, the first two is a re-alloc, to reuse the existing buffer:

SetOfBuffers::One = realloc(OldSet::one, X);

SetOfBuffers::Two = realloc(OldSet::two, X);

The problem?  I'd start to modify the values in the new set of buffers, the third buffer being used and present.  Then the first buffer would be changed present... The second buffer changed present and the information is wrong (I over simplify massively here).

Anyway, I was remotely SSH'd into my server for this, so I went to Visual Studio, same code... Worked fine... So I go into my local VM and it's fine too, so I went back to the server and compiled manually and suddenly it's fine too.... WTF.

I literally spent an hour looking at this, the problem?  Well, it appears to be a bug in Clang, the reason the problem disappeared was my Makefile contains a $CC constant for the compiler to use and it was "clang" when I built by hand I used "g++".  Worse still, if I switched to a clang debug build the code worked fine, so this was something about my compilation process not a bug in the code per se.

So, perplexed I went in search of an answer.  And it appeared to be something about the clang optimizer, about which I found this talk from CppCon 2016.

Where there's this example:

#include <cstdlib>

#include <cstdio>

int main ()

{

int* p = (int*)malloc(4); // The original buffer above

int* q = (int*)realloc(p, 4); // The new pointer to the same old buffer

// Allocate a vlaue

*p = 1;

*p = 2;

if ( p == q )

{

printf("%d %d\n", *p, *q);

}

}

What do you expect this code to display?... Well, I expect it to print "2 2".  And it does on VC and G++ and even clang without the optimizer...

But you optimize the compile and its wrong:

Now, this is undefined behaviour and not caused by your code, it's the optimizer and very scary.  Not least as this was identified a while back (the talk along is from 2016) and g++ has solved the problem... Eeeek.

RIP : Richard Hatch

Having just heard the sad news of the passing of Richard Hatch, I'm genuinely saddened... Perhaps best remembered for being in both the original Battlestar and the fantastic re-imagining... He brought a character of menace and quiet perseverance but recognisable motivation to the latter.

I heard the news about his passing only after seeing RedLetterMedia post a rather wonderful snippet of their talking to him about one of the many schlock Sci-Fi titles he was in, and his laugh, his humour tell you more about the man than perhaps we here in the UK had any real idea of.

But, hit that Turbo button and see the stars now... My Apollo.

Software Engineering : C++14 Factory Create Pattern for GCC

In my previous Software Engineering item, I covered a factory create pattern, to prevent programmers using my classes with their vanilla constructors, forcing them into using a smart pointer in the hope that they would never get a memory leak from using my classes (assuming my classes clean up cleanly internally).

This pattern can be seen here, but is specific to the Microsoft STL implementation of the Memory classes.

On Linux however, with GCC, you have to make a different class a friend, this is "__gnu_cxx::new_allocator", which like "_Ref_count_obj" is an implementation of a counter to references to the allocated class type.  It is also templated, and used exactly the same way as the Microsoft version.

Use it like this:

You can see my whole screen, with all the code here:

Click to Zoom in

On the left is the example code, on the right you see me invoke g++ with C++14 to compile and then run the example program.

The code clearly shows the private constructor, it's a trivial class, if any of that confuses you then this maybe too advanced an item for you to pick up on just yet, but do come back and learn this, as it's very useful when you've built up your C++ confidence.

Software Engineering : C++14 Factory Create Pattern

For the GCC version of this, see this post (at the time of writing this link is to a scheduled item, it may not work just yet, scheduled for Tuesday 4th October 2016 at 18:30 GMT+1).

I commonly employ a "Factory Create" style to my classes in C++, the reason being I like to take control over the specific way that classes can be created, rather than relying on the compiler to always create the default constructors for me.  I like the constructors, but it leads to the problem of memory leaks, lets take a look at a bad example:

With this class any programmer can then do:

Alpha* l_somevalue = new Alpha(42);

They can then forget to call "delete l_somevalue;" and therefore leak memory.

We would therefore like to hide the constructors away, to prevent their miss use:

This necessitates we do expose someway to create an instance of the class, and that instance be wrapped as a smart pointer...

The first thing we see is a forward declaration of the class, then it's type wrapped as a smart pointer (a shared pointer in this case).  Next we see the various constructors have been made public, curing and removing all possibility of the programmer leaving hanging pointers to instances of our class.

Finally, we see a set of static "create" functions, our method to create an instance of the class now would look like this:

BetaPtr l_instance = Beta::Create();

When the reference count to the l_instance copy drops to zero then it will be automatically cleaned up, this is the whole point of this programming pattern.

There is nothing wrong with this code at the moment, we can define the constructors, knowing they can't be used outside the class, like this:

And the create functions look like this:

So far, we've learned nothing new... So what is new?... Well in C++14, really we should not be calling "new" at all.  The moment we see new outside the class we should implement this whole code pattern to hide the constructors away, exposing only smart pointers.  And likewise internally to those create functions we should not be calling "new", we should be using "std::make_shared".

Like this:

The problem?...

Hmmm... "error C2248: 'Beta::Beta': cannot access private memory declared in class 'Beta'", what is this telling us?

That the constructor of Beta could not access one of the private members of itself?  Huh?.. That makes no sense, of course the constructor should be able to access its own member!

What else does the error tell us?  "File: memory"... memory?... Not my code file, but the memory header?  We can open that location and take a look, what is the location of the error representing?

The _Ref_count_obj, the actual location that a shared_ptr is instantiated and the forward passed the arguments to the constructor, so the error is NOT that the constructor of our class can't get at it's private member, but that the template wrapped reference count object, which we inherit from because of the use of "make_shared", that reference can't get at the private members of the class!

How to solve this?... Simple:

Simply adding the "friend std::_Ref_count_obj<Beta>" allows that inherited class from "<memory>" to see inside the private area of the Beta class, and we enable the clean use of std::make_shared thereafter.

I highly recommend removing your constructors from prominence, and the use of "Factory Creation", as to other much more authoritative figures then myself.

My Complete code is below!

#include <iostream>

#include <memory>

class Alpha

{

private:

int m_AlphaValue;

public:

Alpha();

Alpha(const int& p_NewValue);

Alpha(const Alpha& p_OtherAlpha);

const int Value();

void Value(const int& p_NewValue);

};

class Beta;

using BetaPtr = std::shared_ptr<Beta>;

class Beta

{

private:

friend std::_Ref_count_obj<Beta>;

int m_AlphaValue;

Beta();

Beta(const int& p_NewValue);

Beta(const Beta& p_OtherBeta);

public:

static BetaPtr Create();

static BetaPtr Create(const int& p_NewValue);

static BetaPtr Create(const BetaPtr& p_OtherBeta);

const int Value();

void Value(const int& p_NewValue);

};

Beta::Beta()

:

m_AlphaValue(42)

{

}

Beta::Beta(const int& p_NewValue)

:

m_AlphaValue(p_NewValue)

{

}

Beta::Beta(const Beta& p_OtherBeta)

:

m_AlphaValue(p_OtherBeta.m_AlphaValue)

{

}

BetaPtr Beta::Create()

{

return std::make_shared<Beta>();

}

BetaPtr Beta::Create(const int& p_NewValue)

{

return std::make_shared<Beta>(p_NewValue);

}

BetaPtr Beta::Create(const BetaPtr& p_OtherBeta)

{

return std::make_shared<Beta>(*p_OtherBeta.get());

}

int main()

{

Alpha* l_Instance = new Alpha(42);

std::cout << "Alpha: " << l_Instance->Value() << std::endl;

BetaPtr l_BetterInstance = Beta::Create(42);

std::cout << "Beta: " << l_BetterInstance->Value() << std::endl;

// No need to delete l_BetterInstance, it cleans itself up

// Whoops... l_Instance of alpha leaks!

}

Programming : SDL_Window* into std::shared_ptr

I've had a little bit of a brain fart this morning, so to save you all the trouble, I'll go through it... First of all, I am working on an isolated system, therefore I had no internet, no clues, and importantly no books... This is a drag, because mainly I'll check things out look things up and just make sure I get things right... When things go wrong.

Unfortunately, things went wrong, and it was very early (for me) and I was unable to see the problem.

extern "C"

{

    #include <SDL.h>

}

SDL_Window* theWindow = SDL_CreateWindow (

    "The Window Title,

    SDL_WINDOWPOS_UNDEFINED,

    SDL_WINDOWPOS_UNDEFINED,

    800,

    600,

    SDL_WINDOW_SHOWN);

This code will create an SDL window at a unspecified location, with a size of 800x600 and the imaginative title "The Window Title".

This is fine, however, I hate seeing raw pointers, that "theWindow" variable irks me, it's just wrong to see that in modern C++ code; even if I am using a C based library.

So, I wanted to set about wrapping that into a smart pointer:

#include <memory>

namespace Xelous

{

    using SDLWindowPtr = std::shared_ptr<SDL_Window>;

}

This is my wanted type, "SDLWindowPtr", I know what this means to me, and I wanted to have a creating function wrap this for me:

The trouble?... This is considered an incomplete type, when we pass "SDL_CreateWindow" to the smart pointer ctor, it reports "incomplete type".

You can see the full error above, click to zoom in on the image.

So, what have I done wrong?... Well, the brain fart is to forget that "SDL_Window*" does not contain a standard destructor, it actually needs to be passed to SDL_DestroyWindow to close it down & release all resources.

I had totally forgotten this detail, I was too focussing too much upon the creation step and not thinking about the window objects life cycle.  The result was about 20 minutes wasted time banding my head wondering why this was happening.

Two cups of coffee and a pint of water, plus a wander down the corridor later, and I had that moment of realisation about this.

And so my code evolved, giving the solution to create a nullptr smart pointer result, then reset it to use the newly created window and instruct the smart pointer to use the SDL_DestroyWindow call for the clean up.

The complete code for a user might be more like this:

#include <string>

#include <memory>

extern "C"

{

    #include <SDL.h>

}

using SDLWindowPtr = std::shared_ptr<SDL_Window>;

SDLWindowPtr CreateWindowPtr (

    const std::string& p_Title,

    const unsigned int& p_X,

    const unsigned int& p_Y,

    const unsigned int& p_Width,

    const unsigned int& p_Height)

{

    auto l_result = SDLWindowPtr(nullptr);

    l_result.reset (

        SDL_CreateWindow(

            p_Title.c_str(),

            p_X,

            p_Y,

            p_Width,

            p_Height,

            SDL_WINDOW_SHOWN),

        SDL_DestroyWindow);

    return l_result;

}

int main ()

{

    SDLWindowPtr theWindow = CreateWindowPtr(

        "Hello World",

        10,

        10,

        800,

        600);

    SDL_Delay(5000);

}

We don't need to remember to SDL_DestroyWindow, we don't need to worry about forgetting to clean up, the smart pointer will do it for us when the reference count drops to zero.