Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transition from boost::thread to std::thread. #3060

Merged
merged 1 commit into from
May 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Transition from boost::thread to std::thread.
  • Loading branch information
SergioRAgostinho committed May 6, 2019
commit 3d23d5b897e4bab881e4cc63be231b1e07cff7cf
1 change: 0 additions & 1 deletion common/include/pcl/common/boost.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
#include <boost/mpl/size.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <boost/function.hpp>
#include <boost/thread.hpp>
#include <boost/signals2.hpp>
#include <boost/signals2/slot.hpp>
#include <boost/algorithm/string.hpp>
Expand Down
5 changes: 3 additions & 2 deletions common/include/pcl/common/time_trigger.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@
#include <boost/function.hpp>
#include <boost/thread.hpp>
#include <boost/signals2.hpp>
#endif

#include <condition_variable>
#include <mutex>
#endif
#include <thread>

namespace pcl
{
Expand Down Expand Up @@ -101,7 +102,7 @@ namespace pcl
bool quit_;
bool running_;

boost::thread timer_thread_;
std::thread timer_thread_;
std::condition_variable condition_;
std::mutex condition_mutex_;
};
Expand Down
4 changes: 2 additions & 2 deletions common/src/time_trigger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pcl::TimeTrigger::TimeTrigger (double interval, const callback_type& callback)
, condition_ ()
, condition_mutex_ ()
{
timer_thread_ = boost::thread (boost::bind (&TimeTrigger::thread_function, this));
timer_thread_ = std::thread (&TimeTrigger::thread_function, this);
registerCallback (callback);
}

Expand All @@ -63,7 +63,7 @@ pcl::TimeTrigger::TimeTrigger (double interval)
, condition_ ()
, condition_mutex_ ()
{
timer_thread_ = boost::thread (boost::bind (&TimeTrigger::thread_function, this));
timer_thread_ = std::thread (&TimeTrigger::thread_function, this);
}

//////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
8 changes: 4 additions & 4 deletions gpu/kinfu_large_scale/tools/record_maps_rgb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,11 @@ main (int argc, char** argv)
buff.setCapacity (buff_size);
std::cout << "Starting the producer and consumer threads..." << std::endl;
std::cout << "Press Ctrl-C to end" << std::endl;
boost::thread producer (grabAndSend);
std::thread producer (grabAndSend);
std::this_thread::sleep_for(2s);
boost::thread consumer (receiveAndProcess);
boost::thread consumer2 (receiveAndProcess);
boost::thread consumer3 (receiveAndProcess);
std::thread consumer (receiveAndProcess);
std::thread consumer2 (receiveAndProcess);
std::thread consumer3 (receiveAndProcess);
signal (SIGINT, ctrlC);
producer.join ();
{
Expand Down
1 change: 0 additions & 1 deletion io/include/pcl/io/boost.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
#include <boost/numeric/conversion/cast.hpp>
#include <boost/thread/mutex.hpp>
#include <boost/thread/condition.hpp>
#include <boost/thread.hpp>
#include <boost/filesystem.hpp>
#include <boost/bind.hpp>
#include <boost/cstdint.hpp>
Expand Down
4 changes: 3 additions & 1 deletion io/include/pcl/io/davidsdk_grabber.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@

#include <david.h>

#include <thread>

namespace pcl
{
struct PointXYZ;
Expand Down Expand Up @@ -217,7 +219,7 @@ namespace pcl

protected:
/** @brief Grabber thread */
boost::thread grabber_thread_;
std::thread grabber_thread_;

/** @brief Boost point cloud signal */
boost::signals2::signal<sig_cb_davidsdk_point_cloud>* point_cloud_signal_;
Expand Down
4 changes: 3 additions & 1 deletion io/include/pcl/io/depth_sense/depth_sense_device_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@

#include <DepthSense.hxx>

#include <thread>

namespace pcl
{

Expand Down Expand Up @@ -136,7 +138,7 @@ namespace pcl
static boost::mutex mutex_;

/// Thread where the grabbing takes place.
boost::thread depth_sense_thread_;
std::thread depth_sense_thread_;

struct CapturedDevice
{
Expand Down
4 changes: 3 additions & 1 deletion io/include/pcl/io/dinast_grabber.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
#include <libusb-1.0/libusb.h>
#include <boost/circular_buffer.hpp>

#include <thread>

namespace pcl
{
/** \brief Grabber for DINAST devices (i.e., IPA-1002, IPA-1110, IPA-2001)
Expand Down Expand Up @@ -206,7 +208,7 @@ namespace pcl

bool running_;

boost::thread capture_thread_;
std::thread capture_thread_;

mutable boost::mutex capture_mutex_;
boost::signals2::signal<sig_cb_dinast_point_cloud>* point_cloud_signal_;
Expand Down
4 changes: 3 additions & 1 deletion io/include/pcl/io/ensenso_grabber.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@

#include <nxLib.h> // Ensenso SDK

#include <thread>

namespace pcl
{
struct PointXYZ;
Expand Down Expand Up @@ -434,7 +436,7 @@ namespace pcl

protected:
/** @brief Grabber thread */
boost::thread grabber_thread_;
std::thread grabber_thread_;

/** @brief Boost point cloud signal */
boost::signals2::signal<sig_cb_ensenso_point_cloud>* point_cloud_signal_;
Expand Down
5 changes: 3 additions & 2 deletions io/include/pcl/io/hdl_grabber.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include <pcl/point_cloud.h>
#include <boost/asio.hpp>
#include <string>
#include <thread>

#define HDL_Grabber_toRadians(x) ((x) * M_PI / 180.0)

Expand Down Expand Up @@ -293,8 +294,8 @@ namespace pcl
boost::asio::io_service hdl_read_socket_service_;
boost::asio::ip::udp::socket *hdl_read_socket_;
std::string pcap_file_name_;
boost::thread *queue_consumer_thread_;
boost::thread *hdl_read_packet_thread_;
std::thread *queue_consumer_thread_;
std::thread *hdl_read_packet_thread_;
bool terminate_read_packet_thread_;
pcl::RGB laser_rgb_mapping_[HDL_MAX_NUM_LASERS];
float min_distance_threshold_;
Expand Down
9 changes: 5 additions & 4 deletions io/include/pcl/io/openni_camera/openni_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@
#ifdef HAVE_OPENNI

#include <map>
#include <vector>
#include <thread>
#include <utility>
#include <vector>
#include "openni_exception.h"
#include "openni.h"

Expand Down Expand Up @@ -549,9 +550,9 @@ namespace openni_wrapper
boost::condition_variable image_condition_;
boost::condition_variable depth_condition_;
boost::condition_variable ir_condition_;
boost::thread image_thread_;
boost::thread depth_thread_;
boost::thread ir_thread_;
std::thread image_thread_;
std::thread depth_thread_;
std::thread ir_thread_;
};

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion io/include/pcl/io/openni_camera/openni_device_oni.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ namespace openni_wrapper
static void __stdcall NewONIIRDataAvailable (xn::ProductionNode& node, void* cookie) throw ();

xn::Player player_;
boost::thread player_thread_;
std::thread player_thread_;
mutable boost::mutex player_mutex_;
boost::condition_variable player_condition_;
bool streaming_;
Expand Down
4 changes: 3 additions & 1 deletion io/include/pcl/io/real_sense_grabber.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
#include <pcl/point_types.h>
#include <pcl/common/time.h>

#include <thread>

namespace pcl
{

Expand Down Expand Up @@ -265,7 +267,7 @@ namespace pcl
EventFrequency frequency_;
mutable boost::mutex fps_mutex_;

boost::thread thread_;
std::thread thread_;

/// Depth buffer to perform temporal filtering of the depth images
boost::shared_ptr<pcl::io::Buffer<unsigned short> > depth_buffer_;
Expand Down
6 changes: 4 additions & 2 deletions io/include/pcl/io/robot_eye_grabber.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
#include <pcl/point_cloud.h>
#include <boost/asio.hpp>

#include <thread>

namespace pcl
{

Expand Down Expand Up @@ -129,8 +131,8 @@ namespace pcl
boost::asio::ip::udp::endpoint sender_endpoint_;
boost::asio::io_service io_service_;
boost::shared_ptr<boost::asio::ip::udp::socket> socket_;
boost::shared_ptr<boost::thread> socket_thread_;
boost::shared_ptr<boost::thread> consumer_thread_;
boost::shared_ptr<std::thread> socket_thread_;
boost::shared_ptr<std::thread> consumer_thread_;

pcl::SynchronizedQueue<boost::shared_array<unsigned char> > packet_queue_;
boost::shared_ptr<pcl::PointCloud<pcl::PointXYZI> > point_cloud_xyzi_;
Expand Down
2 changes: 1 addition & 1 deletion io/src/davidsdk_grabber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ pcl::DavidSDKGrabber::start ()

frequency_.reset ();
running_ = true;
grabber_thread_ = boost::thread (&pcl::DavidSDKGrabber::processGrabbing, this);
grabber_thread_ = std::thread (&pcl::DavidSDKGrabber::processGrabbing, this);
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion io/src/depth_sense/depth_sense_device_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pcl::io::depth_sense::DepthSenseDeviceManager::DepthSenseDeviceManager ()
try
{
context_ = DepthSense::Context::create ("localhost");
depth_sense_thread_ = boost::thread (&DepthSense::Context::run, &context_);
depth_sense_thread_ = std::thread (&DepthSense::Context::run, &context_);
}
catch (...)
{
Expand Down
2 changes: 1 addition & 1 deletion io/src/dinast_grabber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void
pcl::DinastGrabber::onInit (const int device_position)
{
setupDevice (device_position);
capture_thread_ = boost::thread (&DinastGrabber::captureThreadFunction, this);
capture_thread_ = std::thread (&DinastGrabber::captureThreadFunction, this);
image_ = new unsigned char [image_size_];
}

Expand Down
2 changes: 1 addition & 1 deletion io/src/ensenso_grabber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pcl::EnsensoGrabber::start ()

frequency_.reset ();
running_ = true;
grabber_thread_ = boost::thread (&pcl::EnsensoGrabber::processGrabbing, this);
grabber_thread_ = std::thread (&pcl::EnsensoGrabber::processGrabbing, this);
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
10 changes: 5 additions & 5 deletions io/src/hdl_grabber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ pcl::HDLGrabber::start ()
if (isRunning ())
return;

queue_consumer_thread_ = new boost::thread (boost::bind (&HDLGrabber::processVelodynePackets, this));
queue_consumer_thread_ = new std::thread (&HDLGrabber::processVelodynePackets, this);

if (pcap_file_name_.empty ())
{
Expand Down Expand Up @@ -524,12 +524,12 @@ pcl::HDLGrabber::start ()
PCL_ERROR("[pcl::HDLGrabber::start] Unable to bind to socket! %s\n", e.what ());
return;
}
hdl_read_packet_thread_ = new boost::thread (boost::bind (&HDLGrabber::readPacketsFromSocket, this));
hdl_read_packet_thread_ = new std::thread (&HDLGrabber::readPacketsFromSocket, this);
}
else
{
#ifdef HAVE_PCAP
hdl_read_packet_thread_ = new boost::thread (boost::bind (&HDLGrabber::readPacketsFromPcap, this));
hdl_read_packet_thread_ = new std::thread (&HDLGrabber::readPacketsFromPcap, this);
#endif // #ifdef HAVE_PCAP
}
}
Expand All @@ -538,12 +538,12 @@ pcl::HDLGrabber::start ()
void
pcl::HDLGrabber::stop ()
{
// triggers the exit condition
terminate_read_packet_thread_ = true;
hdl_data_.stopQueue ();

if (hdl_read_packet_thread_ != nullptr)
{
hdl_read_packet_thread_->interrupt ();
hdl_read_packet_thread_->join ();
delete hdl_read_packet_thread_;
hdl_read_packet_thread_ = nullptr;
Expand All @@ -566,7 +566,7 @@ pcl::HDLGrabber::stop ()
bool
pcl::HDLGrabber::isRunning () const
{
return (!hdl_data_.isEmpty () || (hdl_read_packet_thread_ != nullptr && !hdl_read_packet_thread_->timed_join (boost::posix_time::milliseconds (10))));
return (!hdl_data_.isEmpty () || hdl_read_packet_thread_);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you help me to understand what was the point of timed_join and why we don't need it anymore? (I do know that std::thread does not have it, but still.)

Copy link
Member Author

@SergioRAgostinho SergioRAgostinho May 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, indeed we're missing some logic if we just remove everything.

The underlying logic with timed_join was the following: you request the thread to join under a specified time and if it manages to successfully do it, it returns true, otherwise false. If your grabber is running normally, there´s no chance for the thread to join, hence the check for that condition. As you said, we no longer have access to timed_join, but usually all these grabbers have an exit flag - terminate_read_packet_thread_ in this case - which marks the intent to break out of the grabber loop. This exit flag is not a real/pure proxy to check if the grabber is still running or not, but it gives you an indication of what will happen very soon, making it a potentially viable replacement candidate to check. Ultimately, I can have a look on how to fully replace this functionality with a condition_variable. It might actually not be super involved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. With this PR there will be a brief period between stop() is called and the thread has joined during which isRunning() will still return true. I don't think this is a problem. Technically, we are still grabbing if the thread hasn't joined.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this PR there will be a brief period between stop() is called and the thread has joined during which isRunning() will still return true.

To prevent that we can add a check to terminate_read_packet_thread_. This is the first variable to get set to true once stop() is invoked. I see no harm in doing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, but setting that flag does not terminate the grabber thread right away. It may happen that one more frame is read before it has an effect.

Ultimately this comes down to the fact that we don't have a strict definition what "is running" means. Is it when the thread is still active? Is it until stop() is called? But I guess it's not that important. If you prefer, you can add the check you mentioned, then we will adopt the second definition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No preference. Since there's no strict definition and currently no reason to create one I say we could leave as it is.

}

/////////////////////////////////////////////////////////////////////////////
Expand Down
6 changes: 3 additions & 3 deletions io/src/openni_camera/openni_device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,19 +393,19 @@ openni_wrapper::OpenNIDevice::Init ()
//focal length from mm -> pixels (valid for 1280x1024)
depth_focal_length_SXGA_ = static_cast<float> (static_cast<XnDouble> (depth_focal_length_SXGA) / pixel_size);

depth_thread_ = boost::thread (&OpenNIDevice::DepthDataThreadFunction, this);
depth_thread_ = std::thread (&OpenNIDevice::DepthDataThreadFunction, this);
}

if (hasImageStream ())
{
boost::lock_guard<boost::mutex> image_lock (image_mutex_);
image_thread_ = boost::thread (&OpenNIDevice::ImageDataThreadFunction, this);
image_thread_ = std::thread (&OpenNIDevice::ImageDataThreadFunction, this);
}

if (hasIRStream ())
{
boost::lock_guard<boost::mutex> ir_lock (ir_mutex_);
ir_thread_ = boost::thread (&OpenNIDevice::IRDataThreadFunction, this);
ir_thread_ = std::thread (&OpenNIDevice::IRDataThreadFunction, this);
}
}

Expand Down
2 changes: 1 addition & 1 deletion io/src/openni_camera/openni_device_oni.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ openni_wrapper::DeviceONI::DeviceONI (

player_.SetRepeat (repeat);
if (streaming_)
player_thread_ = boost::thread (&DeviceONI::PlayerThreadFunction, this);
player_thread_ = std::thread (&DeviceONI::PlayerThreadFunction, this);
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
4 changes: 2 additions & 2 deletions io/src/pcd_grabber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ pcl::PCDGrabberBase::start ()
}
else // manual trigger
{
boost::thread non_blocking_call (boost::bind (&PCDGrabberBase::PCDGrabberImpl::trigger, impl_));
std::thread non_blocking_call (&PCDGrabberBase::PCDGrabberImpl::trigger, impl_);
}
}

Expand All @@ -416,7 +416,7 @@ pcl::PCDGrabberBase::trigger ()
{
if (impl_->frames_per_second_ > 0)
return;
boost::thread non_blocking_call (boost::bind (&PCDGrabberBase::PCDGrabberImpl::trigger, impl_));
std::thread non_blocking_call (&PCDGrabberBase::PCDGrabberImpl::trigger, impl_);

// impl_->trigger ();
}
Expand Down
2 changes: 1 addition & 1 deletion io/src/real_sense_grabber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ pcl::RealSenseGrabber::start ()
THROW_IO_EXCEPTION ("Invalid stream profile for PXC device");
frequency_.reset ();
is_running_ = true;
thread_ = boost::thread (&RealSenseGrabber::run, this);
thread_ = std::thread (&RealSenseGrabber::run, this);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions io/src/robot_eye_grabber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,8 @@ pcl::RobotEyeGrabber::start ()

terminate_thread_ = false;
resetPointCloud ();
consumer_thread_.reset(new boost::thread (boost::bind (&RobotEyeGrabber::consumerThreadLoop, this)));
socket_thread_.reset(new boost::thread (boost::bind (&RobotEyeGrabber::socketThreadLoop, this)));
consumer_thread_.reset(new std::thread (&RobotEyeGrabber::consumerThreadLoop, this));
socket_thread_.reset(new std::thread (&RobotEyeGrabber::socketThreadLoop, this));
}

/////////////////////////////////////////////////////////////////////////////
Expand Down
Loading