C++ thread does not join even after long waiting

Question

I am trying to test my boost asio socket listener using boost unit test. The purpose of the listener is simply listening on a port and read what ever comes in and save it into a queue and send back a http response header.

As a first step, I have created a client to send messages to a listener and read the response message comes from the listener. I have also created a thread to start the listener. Main thread will send and receive messages to and from the Listener. I could able to send and receive messages between client and listner. But when I try to join it does not join and keeps on waiting to join(I guess).

Please help me what went wrong.

Thanks in advance.

Listener.hpp

#ifndef Listener_hpp
#define Listener_hpp
#include <iostream>
#include <stdio.h>
#include <atomic>
#include <boost/asio.hpp>
#include "sharedQueue.hpp"

using namespace std;
using namespace boost;
using boost::asio::ip::tcp;

class Listener{

private:
int port;
std::atomic<bool> m_stop;
boost::system::error_code error;
boost::asio::io_service io_service;
std::shared_ptr<sharedQueue> queue_ptr;
void saveMessageToQueue(std::string,std::string);
Listener(int portToListen, std::shared_ptr<sharedQueue>  queue_unique_ptr);
Listener(const Listener&);
Listener& operator=(const Listener&);
public:
static Listener& getInstance(int portToListenOn,         std::shared_ptr<sharedQueue>   queue_unique_ptr);
void listen();
};
#endif /* Listener_hpp */

Listener.cpp

 #include "Listener.hpp"
 Listener::Listener(int portToListen, std::shared_ptr<sharedQueue>   queue_unique_ptr):port(portToListen), queue_ptr(queue_unique_ptr),m_stop(false)
 {;}
 Listener& Listener::getInstance(int portToListenOn, std::shared_ptr<sharedQueue>   queue_unique_ptr)
 {
  static Listener instance(portToListenOn,  queue_unique_ptr);
  return instance;
 }
 void Listener::stopListen(){
 m_stop = true;
 }
 void Listener::listen()
 {
  try{
    tcp::acceptor acceptor(io_service, tcp::endpoint(tcp::v4(), port));
    while(!m_stop)
    {
        std::cout<<"Server is listening on port "<<port<<std::endl;
        boost::asio::ip::tcp::socket socket(io_service);
        acceptor.accept(socket);
        std::cout<<"Connection Request Accepted "<<std::endl;
        std::array<char, 512> buf;
        socket.read_some(boost::asio::buffer(buf), error);
        std::string responseBack =" HTTP:300";
        boost::asio::write(socket, boost::asio::buffer(responseBack, responseBack.size()), error);
    }
  }
  catch (std::exception& e)
  {
    std::cerr <<"Listener Exception : "<< e.what() << std::endl;
    exit(0);
  }
  }

sharedQueue.hpp

  #ifndef SharedQueue_hpp
  #define SharedQueue_hpp
  #include <stdio.h>
  #include <iostream>
  class sharedQueue{
  public:
  sharedQueue(){};
  void pushToQueue(std::string str){};
  };
  #endif /* SharedQueue_hpp */

ClientTestHelper.hpp

  #ifndef ClientTestHelper_hpp
  #define ClientTestHelper_hpp

  #include <iostream>
  #include <boost/asio.hpp>
  using namespace std;
  class ClientTestHelper
  {
    string address = "";
    public:
    ClientTestHelper(std::string addressToPush){
    this->address = addressToPush;
   };
  ~ClientTestHelper(){};
  std::string sendMessage(string message);
  };

  #endif /* ClientTestHelper_hpp */

ClientTestHelper.cpp

#include "ClientTestHelper.hpp"
std::string ClientTestHelper::sendMessage(string message){
boost::asio::io_service io_service;
boost::asio::ip::tcp::resolver resolver(io_service);
boost::asio::ip::tcp::resolver::query query(address, boost::asio::ip::resolver_query_base::numeric_service);
boost::asio::ip::tcp::resolver::iterator endpoint_iterator = resolver.resolve(query);
boost::asio::ip::tcp::socket socket(io_service);
boost::asio::connect(socket, endpoint_iterator);
boost::asio::streambuf writeBuffer;
boost::asio::streambuf readBuffer;
std::string str = message;
boost::system::error_code error;
boost::asio::write(socket, boost::asio::buffer(str, str.size()), error);
std::array<char,1024> buf;
socket.read_some(boost::asio::buffer(buf), error);
std::string respo = buf.data();
cout<<respo<<std::endl;
return respo;
}

ListenerTest.hpp

#ifndef ListenerTest_hpp
#define ListenerTest_hpp

#define BOOST_TEST_DYN_LINK
#define BOOST_TEST_MAIN

#include <iostream>
#include <stdio.h>
#include <thread>
#include <boost/test/unit_test.hpp>
#include "Listener.hpp"
#include "ClientTestHelper.hpp"
#include "SharedQueue.hpp"

struct ListenerTestFixture{
public:
void startListen(Listener &receiver){
    receiver.listen();
}
std::shared_ptr<sharedQueue> myQueue  = std::make_shared<sharedQueue>();
Listener &receiver = Listener::getInstance(8282, myQueue);
void doRun(){
    receiver.listen();
}
ClientTestHelper *client = new ClientTestHelper("8282");
ListenerTestFixture(){};
~ListenerTestFixture(){};
};
#endif /* ListenerTest_hpp */

ListenerTest.cpp

#include "ListenerTest.hpp"

BOOST_FIXTURE_TEST_CASE(connectionTest, ListenerTestFixture)
{
std::thread t1(&ListenerTestFixture::doRun, *this);
std::string response = "";
response   = client->sendMessage("SomeRandom Messages \r\n\r\n\r\n");
//receiver.~Listener();
receiver.stopListen();
std::cout<<"Response Received "<<response;
t1.join();
std::cout<<"Finished Testing "<<endl;
}

Console Output For Clarification:

Running 1 test case...
Server is listening on port 8585
Connection Request Accepted 
Server is listening on port 8585
  HTTP:300

After this execution did not stopped instead waiting to join.

EDIT: Modified Listener class added member m_stop and stopListen(), Changed for(;;) into while(!m_stop). Header file included.

Answer 1

receiver.~Listener(); doesn't do what you think it does. That line of code can't work and has to come out.

What it does is cause the destructors to be called on all the members of receiver. It won't (directly) interrupt the call to listen(). The likely outcome is that the thread executing listen() will perform undefined behaviour and almost certainly that means fail to end in a way that notifies join().

In short your main thread will wait forever because you've "pulled the rug out" from under receiver().

Even if that somehow worked the compiler will have added an implicit call of receiver.~Listener() at the end of main() and likely crash there if by chance it makes it that far.

The normal solution is to add a member to Listener such as std::atomic<bool> m_stop; (or if you prefer Boost equivalents if you're using Boost) initialise it to false in the constructor (not listen()) and then call a member called stop() that sets it to true immediately before calling join().

You then replace the non-terminating loop in listen() with while(!stop).

That way you main thread can notify the listening the to come to an orderly conclusion at the end of a loop.

There is no general way of just making a thread "just" or "suddenly" stop cleanly. You can't anticipate what point it is at in a process or what resources it was using. It might be in a library call and corrupt the heap, the operating system, the file system, anything.

As an aside Java has a thread method called stop() it was pretty much deprecated immediately after it was added. There is no safe way to implement it or use it.

---

It is rare that you should ever call a destructor directly. The only use case I know is where you've used placement new and don't want to return the space back to the heap. It's even rarer in modern C because things like recycling space in containers is all handed over to std containers like std::vector<>.

Most implementations of std::vector<> call destructors because if you resize the vector down it needs to destruct the object but not reallocate the internal storage.

You should pretty much never call a destructor on a statically allocated object.

Answer 2

if you are running asio functions like

acceptor.accept(socket);

they will block unless they have finished or return with an error or exception (except their underlying socket has been set to non-blocking mode!). Thus, you do not reach your test if m_stop has been set or not.

you'd need to cancel any running operation inside the stop() function, i.e.:

void Listener::stopListen(){
    std::error_code _ignore;
    m_stop = true;
    m_listener.cancel(_ignore); // pass in an ec, it may throw otherwise
    m_sock.cancel(_ignore);
}

Of course, you'd need to embedd the io objects (socket/acceptor) inside your class, to make them reachable inside stopListen().

However, to be honest, I'd highly recommand to writing that code with the asynchroneous pattern, and use

m_acceptor.async_accept(m_sock, [&](const std::error_code &ec) {
    if (ec) {
        return handle_error(ec, "accept"); // bail out
    }

    handle_new_client(std::move(m_sock)); // move out socket
    // restart listen here
});

The threaded approach of course pulls in additional potential race conditions, which are sometimes hard to handle properly.

Chris Kohlhoff has some echo server examples with different acceptor styles from blocking to async, showing working patterns.

Cheers, A.

S: read_some() will most likely not do what you want. It just returns the first few bytes that already have reached your computers TCP stack. There's however no guarantee that the full request is available, so you'd need to invoke async_read multiple times. If the protocol is http, asio::async_read_until would be better, and use a "\r\n" separator.