Asyncronous Function Calls

Question

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

Answer 1

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.