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

Add an alternate method to enqueueAndDetach a task without requiring the capture of std::future<> #31

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Calthron
Copy link

Add a new alternate method to enqueueAndDetach a task without requiring the capture of std::future<>. Note: There is probably a better name for this method, open to suggestions.

…rn value

Add the ability to enqueue a task that does not require monitoring a
return value.  std::future<T> on destruction is expected to wait() for a
thread to complete before completing destruction (btw, MSVS2103
std::future/std::ansync does not behave according to C++ Standard).  If
a temporary future is created (not captured), the calling thread should
block waiting for the thread to exit and complete the promise.  An
alternate enqueueAndDetach method was added to allow for invocations
that don't require the capturing of the std::future.
Typo in comment
@Calthron Calthron changed the title Master Add additional enqueue method that doesn't require capturing an std::future<T>. Nov 10, 2015
Break the source into header and implementation, make the template
definition more readable by changing F to Callable
Rename ThreadPool.h to ThreadPool.hpp
Comment update
@Calthron Calthron changed the title Add additional enqueue method that doesn't require capturing an std::future<T>. Master Nov 11, 2015
@Calthron Calthron changed the title Master Add an alternate method to enqueueAndDetach a task without requiring the capture of std::future<> Nov 11, 2015
Use typename instead of class
Provide a single exit point for the worker thread
Use lock_t instead of Lock

// Don't allow an enqueue after stopping
if (stop)
throw std::runtime_error ("enqueue on stopped ThreadPool");

Choose a reason for hiding this comment

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

@progschj isn't this going to cause a throw from destructor potentially? that seems to the be only time when "stop" is called. IIRC you can never cleanly catch such an exception -- DTOR throw is undefined behavior.

Copy link
Author

Choose a reason for hiding this comment

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

I’m not sure I understand the question.

This would be throwing on a caller’s thread when the internal ‘stop’ flag is true, and the caller was trying to add a task. If the caller is calling member functions while the object is being destroyed… that would be another sort of issue.

From: peter karasev [mailto:[email protected]]
Sent: Thursday, June 16, 2016 11:56 AM
To: progschj/ThreadPool [email protected]
Cc: Steven LeMay [email protected]; Author [email protected]
Subject: Re: [progschj/ThreadPool] Add an alternate method to enqueueAndDetach a task without requiring the capture of std::future<> (#31)

In ThreadPool.hpp #31 (comment) :

  • // Notify a thread that there is new work to perform
  • condition.notify_one ();
  • return result;
    +}

+// Add a new work item to the pool
+template<typename Callable, typename... Args>
+auto ThreadPool::enqueueAndDetach (Callable&& callable, Args&&... args)

  • -> void
    +{
  • { // Critical section
  •  Lock lock (queue_mutex);
    
  •  // Don't allow an enqueue after stopping
    
  •  if (stop)
    
  •     throw std::runtime_error ("enqueue on stopped ThreadPool");
    

@progschj https://github.com/progschj isn't this going to cause a throw from destructor potentially? that seems to the be only time when "stop" is called. IIRC you can never cleanly catch such an exception -- DTOR throw is undefined behavior.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/progschj/ThreadPool/pull/31/files/f761d527a83a37003ecef53c94718818e60d8601#r67404012 , or mute the thread https://github.com/notifications/unsubscribe/AKJadATwrIy1WhowY4pCzd32mUBhU6JSks5qMZxagaJpZM4GfmQ6 . https://github.com/notifications/beacon/AKJadA15Aixeyyd70laKpBGr_6yKx0KRks5qMZxagaJpZM4GfmQ6.gif

enqueAndDetach is not needed as std::future wait is not an issues on
it's destruction if it is not created from std::async.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants