0

I am trying to make simple server that remembers and operates some variables with receive short instructions. I didn't complete this server, and I am trying to test connecting to the server. But when I try to connect the server, it occurs segmentation fault. It seems that be occured at io_context.run() function. I don't know exact cause of this error in spite of reading asio's reference page. Please help me..

I think that you don't have to read code of data(data.hpp). This is server code.

//server.cpp
#include <iostream>
#include "network/sc_network.hpp"

int main(int argc, char *argv[])
{
    try
    {
        if(argc != 2)
        {
            std::cerr << "Usage: server <port>\n";
            return 1;
        }

        boost::asio::io_context io_context;

        tcp::endpoint endpoint(tcp::v4(), std::atoi(argv[1]));
        server server(io_context, endpoint);

        io_context.run();

    }
    catch (std::exception& e)
    {
        std::cerr << "Exception: " << e.what() << "\n";
    }
    return 0;
}

This is client code.

//client.cpp
#include <iostream>
#include <thread>
#include <cstdlib>
#include <boost/asio.hpp>
#include "network/data/data.hpp"

using boost::asio::ip::tcp;

class client{
private:
    boost::asio::io_context& io_context_;
    tcp::socket socket_;
    oper_data *data_;

    void do_connect(const tcp::resolver::results_type& endpoints)
    {
        boost::asio::async_connect(socket_, endpoints,
            [this](boost::system::error_code ec, tcp::endpoint)
            {
                if(!ec)
                {
                    boost::asio::async_read(socket_,
                        boost::asio::buffer(data_, sizeof(oper_data)),
                        [this](boost::system::error_code ec, std::size_t)
                        {
                            if(!ec)
                            {
                                boost::asio::async_write(socket_,
                                    boost::asio::buffer(data_,sizeof(oper_data)),
                                    [this](boost::system::error_code ec, std::size_t)
                                    {

                                    });
                            }
                            else
                            {
                                socket_.close();    
                            }
                        });
                }

                else
                {
                    socket_.close();
                }
            });
    }
public:
    client(boost::asio::io_context& io_context,
        const tcp::resolver::results_type& endpoints)
        : io_context_(io_context),
        socket_(io_context) 
    {
        do_connect(endpoints);
    }
    void write(const oper_data& data)
    {
        boost::asio::post(io_context_,
        [this, data]()
        {

        });
    }
};

int main(int argc, char *argv[])
{
    try
    {
        if(argc != 3)
        {
            std::cerr << "Usage: client <host> <port>\n";
            return 1;

        }
        boost::asio::io_context io_context;

        tcp::resolver resolver(io_context);
        auto endpoints = resolver.resolve(argv[1], argv[2]);
        client c(io_context, endpoints);

        std::thread t([&io_context](){ io_context.run(); });

        char line[128];

        while (std::cin.getline(line, 128))
        {
            oper_data data;
            //processing the line with deviding in 3 words.
        }
    }
    catch (std::exception& e)
    {
        std::cerr << "Exception: " << e.what() << "\n";
    }

    return 0;
}

this is sc_network.hpp

//sc_network.hpp
#include <boost/asio.hpp>
#include <memory>
#include <utility>
#include "data/data.hpp"

using boost::asio::ip::tcp;

class session
    : public std::enable_shared_from_this<session>
{
private:
    tcp::socket socket_;
    data_proc data_proc_;
public:
    session(tcp::socket socket)
        : socket_(std::move(socket)){}

    void start()
    {
        oper_data *input_data;
        boost::asio::async_read(socket_,
            boost::asio::buffer(input_data, sizeof(oper_data)),
            [this, input_data](boost::system::error_code ec, std::size_t)
            {
                if(!ec)
                {
                    data_proc_.set_data(*input_data);
                    data_proc_.oper_process();
                    start();
                }
                else
                {
                    return;
                }
            });
    }
};

class server
{
private:
    tcp::acceptor acceptor_;

    void do_accept()
    {
        acceptor_.async_accept(
            [this](boost::system::error_code ec, tcp::socket socket)
            {
                if(!ec)
                {
                    session ex_session(std::move(socket));
                }

                do_accept();
            });
    }
public:
    server(boost::asio::io_context& io_context,
        const tcp::endpoint& endpoint)
        : acceptor_(io_context, endpoint)
    {
        do_accept();
    }

};

this is data.hpp.

//data.hpp
#include <deque>
#include <cstring>
#include "favdew_utility.hpp"

#define max_oper_size 5
#define max_oper_buf max_oper_size + 1

struct oper_data {
    char oper_[max_oper_buf] = "\0";
    char *operand_;
    char *oper_num_;
};

typedef struct oper_data oper_data;

class data_store {
private:
    char *var_name_;
    char *var_value_;
public:
    data_store()
        : var_name_(NULL), var_value_(NULL) {}

    data_store(const char *var_name, const char *var_value)
    {
        std::size_t var_name_size = strlen(var_name) + 1;
        var_name_ = new char[var_name_size];
        strncpy(var_name_, var_name, strlen(var_name));

        std::size_t var_value_size = strlen(var_value) + 1;
        var_value_ = new char[var_value_size];
        strncpy(var_value_, var_value, strlen(var_value));
    }
    char *var_name() { return var_name_; }
    char *var_value() { return var_value_; }
    void set_value(const char *var_value) {
        var_value_ = new char[strlen(var_value) + 1];
        strncpy(var_value_, var_value, strlen(var_value));
    }
};

typedef std::deque<data_store> data_queue;

class data_proc {
private:
    oper_data data_;
    data_queue proc_queue;

    void var()
    {
        if (data_store *var = this->get_var(data_.operand_)) {
            var->set_value(data_.oper_num_);
        }
        else {
            data_store input_data(data_.operand_, data_.oper_num_);
            this->proc_queue.push_back(input_data);
        }
    }



    bool sum()
    {
        data_store *var = this->get_var(data_.operand_);
        if ( (var) && isNumber(var->var_value()))
        {
            const int input_data = std::atoi(var->var_value()) +
                std::atoi(this->data_.oper_num_);
            var->set_value(std::to_string(input_data).c_str());
            return true;
        }
        else
            return false;
    }

    bool dif()
    {
        data_store *var = this->get_var(data_.operand_);
        if ((var) && isNumber(var->var_value()))
        {
            const int input_data = std::atoi(var->var_value()) -
                std::atoi(this->data_.oper_num_);
            var->set_value(std::to_string(input_data).c_str());
            return true;
        }
        else
            return false;
    }
public:
    data_proc()
    {
        oper_data input_data;

        //<input_data.oper_> is already initialized with "\0"
        std::memset(input_data.operand_, 0, sizeof(char *));
        std::memset(input_data.oper_num_, 0, sizeof(char *));
    }

    data_proc(const char *oper, const char *operand, const char *oper_num)
    {
        strncpy(data_.oper_, oper, max_oper_size);

        std::size_t operand_size = strlen(operand) + 1;
        data_.operand_ = new char[operand_size];
        strncpy(data_.operand_, operand, strlen(operand));

        std::size_t oper_num_size = strlen(oper_num) + 1;
        data_.oper_num_ = new char[oper_num_size];
        strncpy(data_.oper_num_, oper_num, strlen(oper_num));

    }

    inline void set_data(oper_data data)
    {
        this->data_ = data;
    }

    void set_data(const char *oper, const char *operand, const char *oper_num)
    {
        strncpy(data_.oper_, oper, max_oper_size);

        std::size_t operand_size = strlen(operand) + 1;
        data_.operand_ = new char[operand_size];
        strncpy(data_.operand_, operand, strlen(operand));

        std::size_t oper_num_size = strlen(oper_num) + 1;
        data_.oper_num_ = new char[oper_num_size];
        strncpy(data_.oper_num_, oper_num, strlen(oper_num));
    }

    data_store *get_var(const char *var_name)
    {
        const std::size_t queue_size = this->proc_queue.size();

        for (std::size_t i=0; i < queue_size; i++) {
            if (!strcmp(this->proc_queue[i].var_name(), var_name)) {
                return &proc_queue[i];
            }
        }
        return NULL;
    }


    bool oper_process()
    {
        const char *oper = this->data_.oper_;
        if (!strcmp(oper, "var")) {
            var();
            return true;
        }
        else if (!strcmp(oper, "sum")) {
            sum();
            return true;
        }
        else if (!strcmp(oper, "dif")) {
            dif();
            return true;
        }
        else {
            return false;
        }
    }
};

this is favdew_utility.hpp

#include <string>
#include <cstdlib>

bool isNumber(const char *str)
{
    std::size_t length = strlen(str);

    for (std::size_t i = 0; i < length; i++)
    {
        if (!('0' < str[i] && str[i] < '9'))
            return false;

        continue;
    }
    return true;
}

bool isEmpty(void *buffer)
{
    if (!buffer || *(char *)buffer == '\0')
        return true;
    else
        return false;
}
favdew
  • 21
  • 1
  • 2
  • 5
  • 1
    style: [use algorithms](http://coliru.stacked-crooked.com/a/9bd9fff8934dcf91) or even [use c++](https://en.cppreference.com/w/cpp/string/basic_string/stol). Regardless, this is way too much code. You're asking use "debug this for me". – sehe Sep 04 '18 at 10:23

1 Answers1

1

There are many issues, just pointing out a few:

  1. The declaration

    session ex_session(std::move(socket));
    

    This creates a local (stack) variable that inherits from enable_shared_from_this. Using shared_from_this will be Undefined Behaviour

    Session gets immediately destructed and start() appears to be never called

  2. If session::start() were called, it would fail because it starts an async operation without guarding the lifetime of the session instance:

    boost::asio::async_read(socket_,
        boost::asio::buffer(input_data, sizeof(oper_data)),
        [this, input_data](boost::system::error_code ec, std::size_t) { ....
    

    At the very least you need to capture the shared pointer to the session:

      auto self = shared_from_this();
      boost::asio::async_read(socket_,
        boost::asio::buffer(input_data, sizeof(oper_data)),
        [this, self, input_data](boost::system::error_code ec, std::size_t)
    
  3. Even worse, input_data is never initialized. Again: Undefined Behaviour. Even if you did initialize it, you'd have to manage lifetime; why not make it a member of the session, instead of dynamically allocating (or forgetting to, as you have now)?

    Caution: No, you cannot stack-allocate inside start() not even if you capture it in the lambda, because the async operations will not complete before start() exits.

  4. Same in client: data_ is never initialized. Boom.

  5. Even if you had it correctly allocated, using it as an asio::buffer() treats it as a POD.

    Since, however, data_proc happily aggregates a data_queue which is std::deque<> it obviously IS NOT POD. More Undefined Behaviour.

    What you probably need is to serialize your datastructures, instead of hoping that copying some bytes of memory is going to magically "work". It won't!

    See e.g. sending/receiving a struct in boost::asio

    Note While you're at is, use C++ instead of C? All the raw pointers and char* are complexity that you don't need, and it is handing your dozens of footguns or ends or rope that you're gonna hurt yourself more with.

  6. In client.cpp you have:

    std::thread t([&io_context](){ io_context.run(); });
    
    char line[128];
    
    while (std::cin.getline(line, 128))
    {
        oper_data data;
        //processing the line with deviding in 3 words.
    }
    

    Soooo many things...

  7. data_store is also not POD, but it is also a living memory-leak. All the new-ed memory is never freed.

    Note that, the way it's written, the struct might APPEAR to be POD, but logically it isn't (Rule Of Three). Basically, you wrote it in C, not C++. This foregoes all abstractions that C++ has, and now the compiler cannot tell that the struct refers to non-owned resources.

    Mind you, this gives me the impression that oper_data might have similar issues (though at first I assumed that operand_ and _oper_num are supposed to point inside the fixed-size buffer oper_[])

Summarizing:

You're way ahead of yourself. Start much simpler. Use C++ (std::string, never use new/delete, actually use std::make_shared if you want to enable_shared_from_this).

You'll be much happier. Feel free to come back with simpler questions when you get stuck, ideally the SSCCE would be a (few) dozen or so lines.

sehe
  • 374,641
  • 47
  • 450
  • 633
  • thank you for your long advice.. I have realized that my code have very many problem. But I still can't understand correctly why occurs error at io_context.run() in server.cpp ... Therefore, I will retry to make another more simpler server code for not bothering problem except for boost asio.. – favdew Sep 05 '18 at 02:46
  • Either of the things listed that will cause UB will lead to. ... Undefined Behaviour. If you're lucky that's a segfault. If not, it will kill a puppy or something (worst of all it might seem to work well for 13 years until it launches a nuclear missile) – sehe Sep 05 '18 at 02:51