How to make a thread safe DLL?

[Nico]

Hello,

I am creating a DLL and I would like the application that uses it to be able to use the same instance of my DLL with multiple threads simultaneously. That means that the same value can be set or requested at the same time from different threads and possibly with different values.

In the next two posts there is a simplified version of the source-code that I have.

Now my questions are:
1. Would this be enough to safely use the DLL in a multi-threaded program or do I need to change something to enable this?
2. What is the "DllMain" at the bottom of "Dll_interface.cpp" for and how should I use it?
3. How can I make the callback functions such that every thread using the DLL can get its own callback if desired?

Kind regards, Nico

[Nico]

"Dll_interface.h"

#pragma once

#ifndef __DLL_INTERFACE_H__
#define __DLL_INTERFACE_H__

#include <windows.h>

/** @file
 * @brief This headerfile defines the interface between the program and the DLL.
 *
 * Usage:
 *  1. Include this header file in your project and link to the DLL.
 *  2. Call the initialize function before you attempt to use any other function.
 *  3. Call the terminate function when you are done or before you re-initialize if you need a reset
 *
 * \NOTE For multi-threaded programs, the programmer has to ensure that initialize is called before the threads start using the DLL and that terminate is not called before all threads are done using the DLL.
 */

/* This defines how the client application can access and use the DLL (without name mangling issues).                                           *
 * Note: - stdcall is the standard calling convention for the Microsoft Win32 API.                                                              *
 *       - cdecl is the standard calling convention for C and C++ programs (msdn.microsoft.com)                                                 *
 *       In the first (C#) version of the DLL stdcall was used, in later versions cdecl should be prefered, because it is more reliable when    *
 *       using different versions of (different) compilers.                                                                                     */
#ifdef BUILD_DLL
    #define DLL_EXPORT __declspec(dllexport)    // create a .lib file to simplify linking to the DLL.
#else
    #define DLL_EXPORT __declspec(dllimport)
#endif
#ifdef __cplusplus
#define DLL_EXTERN_C extern "C"                 // use unmangled names
#else
#define DLL_EXTERN_C extern
#endif
#define DLL_CALLBACK __cdecl                    // use the __cdecl naming convention
#define DLL_API __cdecl

/* Type definitions for all available callback functions */
typedef void (DLL_CALLBACK *OnConfigurationReceived)(const char* configuration);
typedef void (DLL_CALLBACK *OnErrorValueChanged)(int errorValue, const char* errorSource, const char* errorMessage);
typedef void (DLL_CALLBACK *OnValueChanged)(int argument_1, int argument_2, int newValue);

/* Definition of the exposed functions */

/** \brief Initialize sets all variables to their default value
 * Before calling this functions, values may be un-initialized/un-defined.
 * After calling this function, all other functions can be called safely.
 *
 * \return 0 upon success or an error value upon failure
 */
DLL_EXTERN_C int DLL_API Initialize();

/** \brief Terminate clears all variables and objects kept in memory
 * Calling a function in the DLL after calling this function may result in un-defined behavior or a crash
 *
 * \return 0 upon success or an error value upon failure
 */
DLL_EXTERN_C int DLL_API Terminate();

/** \brief Callback to obtain information about the configuration after the initialize function has been completed.
 *
 * \param OnConfigurationReceived the function to execute when the configuration is loaded.
 *
 * \return 0 upon success or an error value upon failure
 */
DLL_EXTERN_C void DLL_API AttachConfigurationReceivedEvent(OnConfigurationReceived eventHandler);

/** \brief Callback to get notified if an error occurs.
 *
 * \param eventHandler with three arguments, an error code, a buffer for information about where the error was detected and a buffer for the error message.
 * \return 0 upon success or an error value upon failure
 */
DLL_EXTERN_C void DLL_API AttachErrorValueChangedEvent(OnErrorValueChanged eventHandler);

/** \brief Function to get the version of the DLL as a character array
 *
 * \param char* pointer to the array where the version string should be stored
 * \param int with the capacity of the buffer pointed to by the first argument. After execution, this value will indicate the length of the character string.
 *
 * \return 0 upon success or an error value upon failure
 */
DLL_EXTERN_C int DLL_API GetDllVersion(char * version, int versionCapacity);

/** \brief Function to request changing a value
 *
 * This function requests the change of a value . Upon success, the state will be changed and the
 * OnValueChanged event handler will be called with the new value information.
 *
 * \param int that indicates which value to change
 * \param int that indicates which value to change
 * \param int with the number of the desired state
 * \return 0 upon success or an error value upon failure
 */
DLL_EXTERN_C int DLL_API SetValue(int argument_1, int argument_2, int newValue);

/** \brief Function to get the current value of a specific variable
 *
 * \param int that indicates which value to change
 * \param int that indicates which value to change
 * \param int* pointer to the variable where the current value should be stored.
 * \return 0 upon success or an error value upon failure
 */
DLL_EXTERN_C int DLL_API GetValue(int argument_1, int argument_2, int* currentState);

/** \brief Function to add an event handler to get a callback if a value changes (any value)
 *
 * \param eventHandler  with three arguments, the first two indicate which value was changed, the third argument contains the new value
 * \return 0 upon success or an error value upon failure
 */
DLL_EXTERN_C void DLL_API AttachValueChangedEvent(OnValueChanged eventHandler);

#endif // __DLL_INTERFACE_H__ 

"Dll_interface.cpp"

#include "Dll_interface.h"

// from project files
#include "Model.h"
#include "MyErrorCodes.h"


DLL_EXTERN_C int DLL_API Initialize(const char* communicationIP, int communicationPort, const char* logFilesPath, int logFilesCleanupIntervalInWeeks, int enableLoggingComponentChanges)
{
    return Model::instance()->initialize();
}

DLL_EXTERN_C int DLL_API Terminate()
{
// TODO: Clean up all objects
    return implementation_missing;
}

DLL_EXTERN_C void DLL_API AttachConfigurationReceivedEvent(OnConfigurationReceived eventHandler)
{
    Model::instance()->setCallbackOnConfigurationReceived(eventHandler);
}

DLL_EXTERN_C void DLL_API AttachErrorValueChangedEvent(OnErrorValueChanged eventHandler)
{
    Model::instance()->setCallbackOnErrorValueChanged(eventHandler);
}

DLL_EXTERN_C int DLL_API GetDllVersion(char * version, int versionCapacity)
{
    return Model::instance()->getDllVersion(version, versionCapacity);
}

DLL_EXTERN_C int DLL_API SetValue(int argument_1, int argument_2, int newValue)
{
    return Model::instance()->setValue(argument_1, argument_2, newValue);
}

DLL_EXTERN_C int DLL_API GetValue(int argument_1, int argument_2, int* currentValue)
{
    return Model::instance()->getValue(argument_1, argument_2, currentValue);
}

DLL_EXTERN_C void DLL_API AttachValueChangedEvent(OnValueChanged eventHandler)
{
    Model::instance()->setCallbackOnValueChanged(eventHandler);
}

/*extern "C" DLL_EXPORT */BOOL APIENTRY DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
{
    switch (fdwReason)
    {
        case DLL_PROCESS_ATTACH:
            /// What to do here? Is this a possible replacement for Initialize()?
            break;

        case DLL_PROCESS_DETACH:
            /// What to do here? Is this a possible replacement for Terminate()?
            break;

        case DLL_THREAD_ATTACH:
            /// What to do here?
            break;

        case DLL_THREAD_DETACH:
            /// What to do here?
            break;
    }
    return TRUE;
}

[Nico]

"Model.h"

#ifndef MODEL_H
#define MODEL_H

#define maxNumberOfColumns             100
#define maxNumberOfRows                 100

// from the standard library
#include <string>                               // std::string string.find string.find_last_of string.substr
#include <mutex>                                // std::mutex, std::unique_lock, std::condition_variable
// from the project files
#include "MyErrorCodes.h"                       // definition of the available errorcodes
#include "Dll_interface.h"                      // typedef of the callback functions

/** \brief The Model class contains all data and is responsible for access to the data
 */
class Model
{
    public:
        /** \brief Function to get a pointer to the Model object
         *
         * To ensure that the model is the same for all components using it, it is important to access the model using:
         * Model::instance()->someFunction instead of creating an instance of the Model class (because if you creae an
         * instance of the Model there may also be multiple instances containing different values.
         *
         * \return a pointer to the unique Model object.
         */
        static Model*       instance();

        /** \brief Initialize sets all variables to their default value
         * Before calling this functions, values may be un-initialized/un-defined.
         * After calling this function, all other functions can be called safely.
         *
         * \return 0 upon success or an error value upon failure
         */
         int initialize();

        /** \brief Terminate clears all variables and objects kept in memory
         * Calling a function in the DLL after calling this function may result in un-defined behavior or a crash
         *
         * \return 0 upon success or an error value upon failure
         */
        int terminate();

        /** \brief Function to get the version of the DLL as a character array
         *
         * \param char* pointer to the array where the version string should be stored
         * \param int with the capacity of the buffer pointed to by the first argument. After execution, this value will indicate the length of the character string.
         *
         * \return 0 upon success or an error value upon failure
         */
        int getDllVersion(char* version, int versionCapacity);

        /** \brief Function to request changing a value
         *
         * This function requests the change of a value . Upon success, the state will be changed and the
         * OnValueChanged event handler will be called with the new value information.
         *
         * \param int that indicates which value to change
         * \param int that indicates which value to change
         * \param int with the number of the desired state
         * \return 0 upon success or an error value upon failure
         */
        int setValue(int argument_1, int argument_2, int newValue);

        /** \brief Function to get the current value of a specific variable
         *
         * \param int that indicates which value to change
         * \param int that indicates which value to change
         * \param int* pointer to the variable where the current value should be stored.
         * \return 0 upon success or an error value upon failure
         */
        int getValue(int argument_1, int argument_2, int* currentValue);

        /// Functions called when a callback function should be executed
        void sendErrorValueChangedEvent(int errorCode, std::string source, std::string message);
        void sendValueChangedEvent(int collumn, int row, int value);

        /// Register callback functions
        /** \brief Callback to obtain information about the configuration after the initialize function has been completed.
         *
         * \param OnConfigurationReceived the function to execute when the configuration is loaded.
         *
         * \return 0 upon success or an error value upon failure
         */
        void setCallbackOnConfigurationReceived(OnConfigurationReceived handler);

        /** \brief Callback to get notified if an error occurs.
         *
         * \param eventHandler with three arguments, an error code, a buffer for information about where the error was detected and a buffer for the error message.
         * \return 0 upon success or an error value upon failure
         */
        void setCallbackOnErrorValueChanged(OnErrorValueChanged handler);

        /** \brief Function to add an event handler to get a callback if a value changes (any value)
         *
         * \param eventHandler  with three arguments, the first two indicate which value was changed, the third argument contains the new value
         * \return 0 upon success or an error value upon failure
         */
        void setCallbackOnValueChanged(OnValueChanged handler);

    protected:
        // Prevent calling the constructor directly
        Model();
        // Prevent calling the destructor on a singleton class
        virtual ~Model();
    private:
        // Member variables
        std::string             configurationString;
        std::mutex              valuesMutex;
        int                     values[maxNumberOfColumns][maxNumberOfRows];
        static Model*           mModel;

        // Helper function to safely copy a string into a provided buffer
        int getSafeString(const char* result, char* buffer, int buffLength);

        // Places to store the callback functions.
        void (*theConfigurationReceivedHandler)(const char*);
        void (*theOnErrorValueChangedHandler)(int errorValue, const char* errorSource, const char* errorMessage);
        void (*theOnValueChangedHandler)(int, int, int);
};
#endif // MODEL_H 

[Nico]

"Model.cpp"

#include "Model.h"

Model* Model::mModel = nullptr;

/** \brief Constructor of the Model object
 * \NOTE This constructor should not be called, instead the instance() function should be called to ensure that there can always be only one instance of the Model.
 */
Model::Model()
{

}

Model* Model::instance()
{
    if (mModel == nullptr) mModel = new Model();
    return mModel;
}

Model::~Model()
{
}

int Model::initialize()
{
    for (int x = 0; x<maxNumberOfColumns; x++)
    {
        for (int y = 0; y<maxNumberOfRows; y++)
        {
            values[x][y] = 0;
        }
    }
    configurationString = "Configured with " + std::to_string(maxNumberOfColumns) + "x" + std::to_string(maxNumberOfRows) + " matrix.";
    if (theConfigurationReceivedHandler != nullptr) theConfigurationReceivedHandler(configurationString.c_str());
    return success;
}

int Model::terminate()
{
    if (values != nullptr)
    {
        for (int x = 0; x<maxNumberOfColumns; x++)
        {
            if (values[x] != nullptr) delete[] values[x];
        }
        delete values;
    }
    return success;
}

int Model::setValue(int argument_1, int argument_2, int newValue)
{
    if (argument_1 >= 0 && argument_1<maxNumberOfColumns)
    {
        if (argument_2 >= 0 && argument_2<maxNumberOfRows)
        {
            // value protected by mutex to prevent race conditions
            std::unique_lock<std::mutex> valuesGuard(valuesMutex);
            // set the value
            values[argument_1][argument_2] = newValue;
            valuesGuard.unlock();
            // send the callback notification
            sendValueChangedEvent(argument_1, argument_2, newValue);
            return success;
        }
    }
    return unexpected_argument_value;
}

int Model::getValue(int argument_1, int argument_2, int* currentValue)
{
    if (argument_1 >= 0 && argument_1<maxNumberOfColumns)
    {
        if (argument_2 >= 0 && argument_2<maxNumberOfRows)
        {
            // value protected by mutex to prevent updating while getting the value
            std::lock_guard<std::mutex> valuesGuard(valuesMutex);
            *currentValue = values[argument_1][argument_2];
        }
    }
}

int Model::getDllVersion(char* version, int versionCapacity)
{
    return getSafeString("This is DLL version 0.1", version, versionCapacity);
}

void Model::setCallbackOnConfigurationReceived(OnConfigurationReceived handler)
{
    theConfigurationReceivedHandler = handler;
    if (configurationString.size() > 2) theConfigurationReceivedHandler(configurationString.c_str());
}

void Model::setCallbackOnErrorValueChanged(OnErrorValueChanged handler)
{
    theOnErrorValueChangedHandler = handler;
}

void Model::setCallbackOnValueChanged(OnValueChanged handler)
{
    theOnValueChangedHandler = handler;
}

void Model::sendErrorValueChangedEvent(int errorCode, std::string source, std::string message)
{
    if (theOnErrorValueChangedHandler != nullptr)
    {
        theOnErrorValueChangedHandler(errorCode, source.c_str(), message.c_str());
    }
}

void Model::sendValueChangedEvent(int column, int row, int newValue)
{
    if (theOnValueChangedHandler != nullptr) theOnValueChangedHandler(column, row, newValue);
}

/** \brief Function to safely copy a string to a buffer
 *
 * \param const char* the string to be copied
 * \param char* pointer to the buffer where the string should be stored
 * \param int the size of the buffer
 * \return 0 upon success or an error code upon failure.
 */
int Model::getSafeString(const char* result, char* buffer, int buffLength)
{
    if (buffLength < (int)strlen(result) + 1)
    {
        for (int i = 0; i < buffLength; i++) buffer[i] = result[i];
        buffer[buffLength - 1] = '\0';
        return unexpected_argument_value;
    }
    strncpy(buffer, result, strlen(result));
    buffer[strlen(result)] = '\0';
    return success;
}

I apologize to anyone that feels this source code is too long. It is actually only long because there are a lot of comments and i think if you are asking a question, you should provide as much relevant information about what you are trying to do as possible.

[NoXzema]

I apologize to anyone that feels this source code is too long. It is actually only long because there are a lot of comments and i think if you are asking a question, you should provide as much relevant information about what you are trying to do as possible.

Except most of what was posted wasn't even relevant.

I personally prefer a context/object based approach. Your initialize function passes some context that it allocated and readied for you, then the rest of the functions work based on this object passed back.

If you feel that the global approach is the way to go, I hope you're well versed in atomics and multi-threaded resource sharing.

Even in the context/object based approach, you still have to be careful about passing the same object to functions from different threads. You can usually let the user worry about that.

The reason for things like DLL_THREAD_DETACH and DLL_THREAD_ATTACH is mainly to initialize TLS globals. They are called when a thread is created in the process and the DLL has to act accordingly.

[Nico]

Thank you for your quick reply.

The DLL should have a "C-interface", to prevent problems with name mangling when different compilers are used for the program that should use the DLL (or so I have been told). For as far as I know that means the DLL interface can't pass or receive objects as objects only exist in C++ and not in C.
This caused me to end up with my current approach. I made Model an atomic container object because I wanted to go from the global scoped C interface to an object and didn't know another way to do this.
If there is another way, I would love to hear how.

[NoXzema]

This isn't true. Even if it was, you don't really want to be passing back C++ objects to begin with if you can help it (so you immediately allow the use of any language that supports the common C ABI to implement the interface).

You want to keep the object passed back from the interface to be abstract. The user shouldn't be able to understand what's inside of it (at least, not all of it). I would look into opaque pointers to accomplish this.

I don't really like linking to my own code but I have a working example here: https://github.com/computerquip/cqirc/blob/master/src/irc-client.h

That simple interface is implemented completely in C++. "irc_session" and "irc_service" is actually a C++ class. The user doesn't need to know that and he never will. As a matter of fact, the user has no clue of the contents of either of those types. All the user knows is that the interface passes back a pointer to it and you need to pass a pointer of it to the interface if you want something done.

[Nico]

Hello,

I think I see what you did there (took me a while). So I checked, but I was told in this case I am not allowed to change the interface. It turns out there is already software using the interface with a single thread version of the DLL and that should remain compatible.

Personally I think that is a shame, because it would indeed make things a lot easier and cleaner if I could pass complete objects.

Kind regards, Nico