-1
class WIFITest
{
public:

  void StartTest();

  void Notify_Test(boost::shared_ptr<basic_msg> basic_msg, ID id );


private:
  void OpenStaMode_test();

private: 
  boost::shared_ptr<boost::thread> OpenStaMode_testThread;

  boost::shared_ptr<basic_msg> VMF_WIFI_NOTIFY_CLIENT_OPEN_MSG;  

  typedef boost::shared_ptr<boost::thread> THREAD;
  typedef boost::shared_ptr<basic_msg> MSG;
  std::map<ID,std::pair<THREAD,MSG>> map;
};  

void WIFITest::StartTest()
{ 
  map[ID::WIFI_REQ_CLIENT_OPEN] = std::make_pair(OpenStaMode_testThread,VMF_WIFI_NOTIFY_CLIENT_OPEN_MSG);

  this->OpenStaMode_testThread.reset(new boost::thread(&WIFITest::OpenStaMode_test,this));
  this->OpenStaMode_testThread->join();

}
void WIFITest::OpenStaMode_test()
{
  unsigned char strData[WIFI_SEND_BUF_SIZE_MIN];
  sendOpenReq(strData);
  try { 
    boost::this_thread::sleep(boost::posix_time::seconds(8));
  }
  catch(const boost::thread_interrupted&) {   

        std::cout<<"OpenStaMode_test success@ \n";

      }
    else {
        goto OpenStaMode_test_fail;
      }

    return;

  }
OpenStaMode_test_fail:
  printf("@WIFITest::OpenStaMode_test FAIL@@@@@@@@@@@@@@ \n");

}

void WIFITest::Notify_Test(boost::shared_ptr<basic_msg> basic_msg, ID id)
{
  map[id].second = basic_msg;

   map[id].first->interrupt();
  printf("@WIFITest::INTERRUPTED @@@@@@@@@@@@@@ \n");

}

First main will create StartTest instance and call StartTest(), When other code call Notify_Test, "map[id].first->interrupt();" this sentence will produce following assertion fail, and crash the program.

TestWIFI: ../../boost_1_55_0/boost/smart_ptr/shared_ptr.hpp:653: typename boost::detail::sp_member_access::type boost::shared_ptr::operator->() const [with T = boost::thread, typename boost::detail::sp_member_access::type = boost::thread*]: Assertion `px != 0' failed.

ROMANIA_engineer
  • 54,432
  • 29
  • 203
  • 199
UNSTABLE
  • 393
  • 1
  • 4
  • 13

1 Answers1

1

This line:

map[ID::WIFI_REQ_CLIENT_OPEN] = std::make_pair(OpenStaMode_testThread,VMF_WIFI_NOTIFY_CLIENT_OPEN_MSG);

stores into the map a copy of OpenStaMode_testThread, which, at this point, is empty.

Then you call reset(new boost::thread(&WIFITestSuite::OpenStaMode_test,this)) on OpenStaMode_testThread, but that doesn't affect the copy already stored into the map.

As a result, when you call map[id].first->interrupt();, you are calling it on an empty shared_ptr, causing the assertion to fail.


As discussed below, there is a race condition here if a call to Notify_Test can occur after the thread has started execution but before the reset() call on the shared_ptr stores it into the map. To fix this, use a mutex:

class WIFITest
{
public:
  /* ... */
private:
  /* ... */

private: 
  boost::mutex mutex_;
  /* ... */
};  



void WIFITest::StartTest()
{ 
  {
    boost::lock_guard<boost::mutex> guard(mutex_);  // Takes ownership of the mutex
    map[ID::WIFI_REQ_CLIENT_OPEN] = std::make_pair(THREAD(), VMF_WIFI_NOTIFY_CLIENT_OPEN_MSG);
    map[ID::WIFI_REQ_CLIENT_OPEN].first.reset(new boost::thread(&WIFITest::OpenStaMode_test,this));
    // At this point the thread has started and the pointer has been stored into the map
  }  // guard is destructed and mutex released here 
     // so that we don't hold the mutex while waiting
     // for the thread to finish.

  map[ID::WIFI_REQ_CLIENT_OPEN].first->join();
}

/* ... */

void WIFITest::Notify_Test(boost::shared_ptr<basic_msg> basic_msg, ID id)
{
  boost::lock_guard<boost::mutex> guard(mutex_); // Takes ownership of the mutex
  map[id].second = basic_msg;
  map[id].first->interrupt();
  printf("@WIFITest::INTERRUPTED @@@@@@@@@@@@@@ \n");

}
T.C.
  • 133,968
  • 17
  • 288
  • 421
  • Thank you, how to modify my code? I need first store the OpenStaMode_testThread in map. – UNSTABLE Jun 25 '14 at 06:42
  • I need match the id and thread before starting the OpenStaMode_testThread – UNSTABLE Jun 25 '14 at 06:48
  • @huskarwang Why would you need to store an empty `shared_ptr` in the map?? Also, is there any guarantee on when `Notify_Test` will be called? If the answer is "any time after the thread starts", then there's a race condition here. – T.C. Jun 25 '14 at 06:57
  • @ T.C. something like this? Then i do not need OpenStaMode_testThread as member anymore?map[WIFI_REQ_ID::WIFI_REQ_CLIENT_OPEN].first.reset(new boost::thread(&WIFITestSuite::OpenStaMode_test,this)); – UNSTABLE Jun 25 '14 at 06:59
  • I don't want store empty shared_ptr, I want pointer in map and pointer to interrupt be same pointer. – UNSTABLE Jun 25 '14 at 07:00
  • @huskarwang Yes, you can do that, in which case you shouldn't need `OpenStaMode_testThread` as a member unless you are using it somewhere else. However, you have a race condition if `Notify_Test` can be called between the time the new thread is created and the time the `reset()` call stores the pointer to the `boost::thread` object into the `shared_ptr`. – T.C. Jun 25 '14 at 07:03
  • @T.C.What? I think new thread is created in reset(). This sentence will create a new thread, , am i right? map[ID::WIFI_REQ_CLIENT_OPEN].first.reset(new boost::thread(&WIFITest::OpenStaMode_test,this)); – UNSTABLE Jun 25 '14 at 07:09
  • `new boost::thread(&WIFITest::OpenStaMode_test,this)` constructs a new thread and returns a pointer to it, and once it returns the thread may begin executing. This pointer is then passed as a parameter to `reset` which stores it in the `shared_ptr`. Do you see the possible race now? – T.C. Jun 25 '14 at 07:11