1

I'm writing a multi-threaded server using boost::asio (for sockets), boost::thread (for threading), libconfig++ (for configuration files reading) and protocol buffers (for the protocol implementation).

The server follows more or less this route: main() -> creates an Application object -> runs application object. Application loads configuration file, then creates the server object (which is passed the configuration class as a const). Server object configures itself and binds the port, starts accepting, blah. Whenever a new client is detected, the server creates a new Client object, and then creates a thread running the client's connection handler.

All of this is to explain that the configuration file is loaded from my Application class, and then passed all the way down to my Client class. This shouldn't pose any kind of trouble if the libconfig object was passed directly all the way to the Client, yet as we all know, multi-threading implies that memory corrupts when accessed simultaneously by two or more threads.

The way to solve this was discussed in other post and ended up with the implementation of a wrapper which automagically solves the mutex problem.

The magical class

app_config.h

#ifndef _APP_CONFIG_H_
#define _APP_CONFIG_H_ 1

#include <boost/shared_ptr.hpp>
#include <boost/thread/mutex.hpp>
#include <boost/thread/locks.hpp>
#include <boost/noncopyable.hpp>
#include <libconfig.h++>
#include <string>

namespace BBCP {
    namespace App {

class ConfigLock;

class Config {
public:
    friend class BBCP::App::ConfigLock;

    Config(std::string const &file) :
        cfg(new libconfig::Config()),
        mutex(new boost::mutex())
    {
        cfg->readFile(file.c_str());
    }

private:
    boost::shared_ptr<libconfig::Config> cfg;
    boost::shared_ptr<boost::mutex> mutex;
};

class Server;
class Client;

class ConfigLock : boost::noncopyable {
public:
    ConfigLock(BBCP::App::Config const &wrapper) :
        cfg(wrapper.cfg),
        mutex(wrapper.mutex),
        lock(new LockType(*mutex))
    { }

    libconfig::Config &get() throw() { return *cfg; };
private:
    boost::shared_ptr<libconfig::Config> cfg;
    boost::shared_ptr<boost::mutex> mutex;

    typedef boost::lock_guard<boost::mutex> LockType;
    boost::shared_ptr<LockType> lock;
};

    }
}

#endif

For lazy people, this class consists of... well, two classes (irony?): BBCP::App::Config and BBCP::App::ConfigLock. BBCP::App::Config simply loads a file, while BBCP::App::ConfigLock takes a BBCP::App::Config as an argument, and then locks BBCP::App::Config's mutex. Once the lock has been created, one calls BBCP::App::ConfigLock::get, which returns a reference to the libconfig::Config object!.

THE problem

Well:

server.cpp:

void BBCP::App::Server::startAccept() {
    newClient.reset(new BBCP::App::Client(io_service, config_wrapper));
    acceptor.async_accept(newClient->getSocket(), boost::bind(&BBCP::App::Server::acceptHandler, this, boost::asio::placeholders::error));
}

This function creates a new client object, loaded with the boost::asio::io_service object and BBCP::App::Config object.

server.cpp

void BBCP::App::Server::acceptHandler(boost::system::error_code const &e) {
    if (!acceptor.is_open()) {
        // ARR ERROR!
        return;
    }

    if (!e) {
        client_pool.create_thread(*newClient);
    }
    else {
        // HANDLE ME ERROR
        throw;
    }

    startAccept();
}

This function creates a new thread or (not implemented yet) errors in case of... well, errors, then starts the accept loop again.

Client code mostly doesn't matter until this part:

client.cpp:

void BBCP::App::Client::parseBody() {
    BBCP::Protocol::Header header;
    BBCP::Protocol::Hello hello;
    boost::scoped_ptr<BBCP::App::ConfigLock> lock;
    libconfig::Config *cfg;

    (...)

    switch ((enum BBCP::Protocol::PacketType)header.type()) {
        case BBCP::Protocol::HELLO:
            (...)

            // config_wrapper is a private variable in the client class!
            lock.reset(new BBCP::App::ConfigLock(config_wrapper));

            // ARRRRRRR HERE BE DRAGOONS!!
            *cfg = lock->get();

            (...)

            lock.reset();
            break;
        (...)

    }

    (...)
}

Well, truth be told, I didn't expect this kind of error:

/usr/include/libconfig.h++: In member function ‘void BBCP::App::Client::parseBody()’:
/usr/include/libconfig.h++:338:13: error: ‘libconfig::Config& libconfig::Config::operator=(const libconfig::Config&)’ is private
client.cpp:64:30: error: within this context
client.cpp:71:21: error: request for member ‘exists’ in ‘cfg’, which is of non-class type ‘libconfig::Config*’
client.cpp:77:51: error: request for member ‘lookup’ in ‘cfg’, which is of non-class type ‘libconfig::Config*’

But here it is, and I need some way to solve it :(. I've tried making BBCP::App::Client a friend class of BBCP::App::ConfigLock, but then it went like:

In file included from ../include/app_config.h:4:0,
                 from ../include/app_main.h:6,
                 from main.cpp:18:
../include/app_client.h:15:53: error: ‘BBCP::App::Config’ has not been declared
In file included from ../include/app_config.h:4:0,
                 from ../include/app_main.h:6,
                 from main.cpp:18:
../include/app_client.h:32:5: error: ‘Config’ in namespace ‘BBCP::App’ does not name a type
In file included from ../include/app_config.h:4:0,
                 from ../include/app_main.h:6,
                 from main.cpp:18:
../include/app_client.h: In constructor ‘BBCP::App::Client::Client(boost::asio::io_service&, const int&)’:
../include/app_client.h:15:120: error: class ‘BBCP::App::Client’ does not have any field named ‘config_wrapper’

And then I went like O_O, so I just gave up and came here, once again looking for some über C++ guru hackz0r's help and scolding for doing such a misdeed as trying to access another class's private members is.

Community
  • 1
  • 1
Misguided
  • 1,302
  • 1
  • 14
  • 22
  • I don't understand this part: "I've tried making `BBCP::App::Client` a friend class of `BBCP::App::ConfigLock`". The error message is complaining about a `BBCP::App::Client` member function trying to access a private method of `libconfig::Config` (an overloaded assignment operator), so for the friend-class approach to work, wouldn't you need to make `BBCP::App::Client` a friend class of `libconfig::Config`? – ruakh Jan 25 '12 at 03:07
  • 1
    Small advice, rename `_APP_CONFIG_H_`. §17.4.3.2.1 says `Each name that contains a double underscore (_ _) or begins with an underscore followed by an uppercase letter (2.11) is reserved to the implementation for any use.` – jweyrich Jan 25 '12 at 03:10
  • Multithreaded *readonly* access to a variable is not problematic. I don't know whether you intend on modifying the configuration object while the program is running but if you don't everything will be simpler. Also, you should spend some time in the design (why is the config object not publicly copyable) and learning what the compiler is trying to tell you in the error messages, as they are there to help you figure out what the problems are. – David Rodríguez - dribeas Jan 25 '12 at 03:15
  • @ruakh: Yeah, I'm sorry, I misread the error. – Misguided Jan 25 '12 at 05:04
  • @jweyrich: Thanks, where can I get a copy of that?... – Misguided Jan 25 '12 at 05:05
  • @DavidRodriguez: memory will be modified, that is why I'm implementing all this. – Misguided Jan 25 '12 at 05:14
  • 1
    @JulianBayardo: check [Where do I find the current C or C++ standard documents?](http://stackoverflow.com/q/81656/298054). – jweyrich Jan 25 '12 at 06:00

2 Answers2

1

The first thing is figuring out if you are going in the right direction, and the next step is getting there.

Why is the assignment operator of the Config type private? By default the compiler generated assignment operator is public, so if it has been declared as private, chances are that there is a reason for the object not to be copied, or else you should make it public and the problem would no longer be a problem.

As of your particular problem after adding the friend declaration, it seems to indicate that you have missed including the header where the Config type is declared/defined. And then there are some more errors in the code (a member that has not been defined - result of the previous error?), or in the original code trying to access the object referred by a pointer without dereferencing it...

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
  • The idea was not to copy the object, but to store the pointer to the object. Still, I have no idea of why the copy operator was made private. – Misguided Jan 25 '12 at 05:08
0

You probably wanted to store a pointer to the config object in cfg instead of creating a copy (and dereferencing an uninitialized pointer):

cfg = &local->get();
sth
  • 222,467
  • 53
  • 283
  • 367