4
\$\begingroup\$

I am writing a node.js inspired C++ library for asynchronous function calls.

How efficient and stable does my code look?

If you want, you can see some WIP code and demonstration code here. (net.hpp is for POSIX sockets and is far from done. It does not have any IO functions written)

/* ASYNC.HPP
 * Defines some functions for calling other functions in the background
 * This gives the ability to create callback based functions easily,
 * similar to node.js (But I hate javascript so I wrote this library for c++)
 */

#ifndef ASYNC_H
#define ASYNC_H
#include <thread>
#include <vector>

std::vector<std::thread> asyncCalls; // Don't have to deal with threads leaving scope since this vector is global
#define asyncCall asyncCalls.emplace_back // Whenever someone calls asyncCall this constructs a new std::thread which calls their function

void finishAsync(){ //Call to block until all running asyncronous functions return
while(!asyncCalls.empty()){ //Loop until the vector is empty
        asyncCalls.back().join(); //Get the thread from the back and join it to block
                //^ I feel like this line might throw an exception, but in my testing it hasn't thrown anything.
        asyncCalls.pop_back(); //Pop it and get a new one
}
} // Infinite running threads will block forever

#ifdef _GLIBCXX_CHRONO //Include <chrono> before this header for this function

void sleep(uint32_t millis){ //I just realized while adding comments that I could do this with a define. Oh well
    std::this_thread::sleep_for(std::chrono::milliseconds(millis));
}

#endif // End chrono function

#endif //End header guard
\$\endgroup\$
  • 2
    \$\begingroup\$ One thing to note is that node.js is single threaded. The asynchronous nature is achieved by switching to another task only when the current task would block. You need to do research on select(), pselect(), poll(), libEvent(). On the language side if you still want to do it with threads you need to learn about hire level features std::future, std::promise and std::async \$\endgroup\$ – Martin York Apr 22 '17 at 15:30
6
\$\begingroup\$
std::vector<std::thread> asyncCalls;

This creates a global variable (so, you know, don't do that) — and it creates the global variable in every .cpp file that imports this header file. So unless you only have one .cpp file in your project, you're going to get linker errors when you try to link your project.

What you wanted to do was put this variable definition in a .cpp file and put a declaration of it (using extern) in your .hpp file.

Alternatively, as of C++17, you could have made it an inline variable:

inline std::vector<std::thread> asyncCalls;

However, global variables are terrible; don't do anything like this. Maybe what you want is some notion of "thread pool":

class ThreadPool {
    std::vector<std::thread> asyncCalls;
    // ...
};

#define asyncCall asyncCalls.emplace_back // Whenever someone calls asyncCall this constructs a new std::thread which calls their function

This is a preprocessor macro (so, you know, don't do that). What you meant was

class ThreadPool {
    std::vector<std::thread> asyncCalls;

public:
    template<typename... Args>
    void asyncCall(Args&&... args) {
        asyncCalls.emplace_back(std::forward<Args>(args)...);
    }
};

void finishAsync(){ //Call to block until all running asyncronous functions return
while(!asyncCalls.empty()){ //Loop until the vector is empty

Two things:

  • Please indent your code. Four-space indents would be nice.

  • Who do you expect to call this function? and how do you know that nobody else is going to be calling emplace_back at the same instant that that guy is calling empty? Looks like you have a huge thread-safety problem here.


#ifdef _GLIBCXX_CHRONO //Include <chrono> before this header for this function

Don't do this. For one thing, your code won't do the right thing on Clang with libc++ (or MSVC, or basically any non-libstdc++ distribution). What you meant was

#include <chrono>

This way, your header includes all the headers it depends on, recursively, and you never have to worry about whether your caller included something else before you or not.


As far as "would this code be a good idea if it worked": a resounding no. Your asyncCall function isn't really doing anything that std::async wouldn't do on its own; and std::async on its own is a bad idea because if you call it too many times you'll eventually run out of threads and start getting "Resource temporarily exhausted" errors. If you're going to be writing programs that use asynchrony as their primary implementation technique, the last thing you want to do is run out of threads. You need to figure out some way to queue up "tasks" without tying up a whole std::thread per (stalled) task.

\$\endgroup\$
  • \$\begingroup\$ I don't want to include the whole chrono library just for that one function though. Is there a way for me to check if chrono is included without using _GLIBCXX_CHRONO? I will look at std::async. \$\endgroup\$ – user233009 Apr 22 '17 at 15:51
  • 1
    \$\begingroup\$ "I don't want to include the whole chrono library just for that one function though." — First, don't worry about it; but second, if you did have a real reason to worry about it, the appropriate thing to do would be to move your own sleep function to a separate header, and/or to define it (and #include <chrono>) only when #if DECLARE_MY_SLEEP_FUNCTION. Make control paths explicitly controlled by the user, not implicitly controlled by the order of #includes in some other source file. \$\endgroup\$ – Quuxplusone Apr 22 '17 at 19:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.