I am working on testing a lock-free work-stealing queue I wrote recently. However, I found a weird issue, that when I push a new item to the queue, it only returns me the same object that was pushed at the last. It took me one day and I still cannot figure out why this happened. This is the output, every time, the data is the same to be pushed to the queue, which is why I only have the last result being 45, it should be all 45 for the 4 res items. Anybody can help me here :(
Below is the simplified code:
#include <memory>
#include <functional>
#include <type_traits>
class FunctionWrapper {
private:
class ImplInterface {
public:
virtual void invoke() = 0;
virtual ~ImplInterface() {}
};
std::unique_ptr<ImplInterface> impl;
template <typename F, typename... Args>
class Impl : public ImplInterface {
private:
std::function<void()> callBack;
public:
Impl(F&& f_, Args&&... args_) {
callBack = [&f_, &args_...]() { f_(std::forward<Args>(args_)...); };
}
void invoke() override { callBack(); }
};
public:
template <typename F, typename... Args>
FunctionWrapper(F&& f_, Args&&... args_) : impl(new Impl<F, Args...>(std::forward<F>(f_), std::forward<Args>(args_)...)) {}
void operator()() { impl->invoke(); }
FunctionWrapper() = default;
FunctionWrapper(FunctionWrapper&& other) : impl(std::move(other.impl)) {}
FunctionWrapper& operator=(FunctionWrapper&& other) {
impl = std::move(other.impl);
return *this;
}
FunctionWrapper(const FunctionWrapper&) = delete;
FunctionWrapper(FunctionWrapper&) = delete;
FunctionWrapper& operator=(const FunctionWrapper&) = delete;
};
#include <atomic>
#include <array>
#include <iostream>
#include "functionwrapper.h"
class LockFreeWorkStealingQueue {
private:
using DataType = FunctionWrapper;
static constexpr auto DEFAULT_COUNT = 2048u;
static constexpr auto MASK = DEFAULT_COUNT - 1u;
std::array<DataType, DEFAULT_COUNT> q;
unsigned int ft{0};
unsigned int bk{0};
public:
LockFreeWorkStealingQueue() {}
LockFreeWorkStealingQueue(const LockFreeWorkStealingQueue&) = delete;
LockFreeWorkStealingQueue& operator=(const LockFreeWorkStealingQueue&) = delete;
void push_back(DataType data) {
std::cout << "bk: " << (bk&MASK) << " &data: " << &data << std::endl;
q[bk & MASK] = std::move(data);
bk++;
}
bool try_pop_back(DataType& res) {
if (bk > ft) {
res = std::move(q[(bk - 1) & MASK]);
bk--;
return true;
}
return false;
}
};
#include "lockfreeworkstealingqueue.h"
#include <iostream>
#include <algorithm>
#include <numeric>
#include <cassert>
#include <vector>
constexpr unsigned int NUM = 4;
void sumOver(const std::vector<int>& v, int& res) {
res = std::accumulate(v.begin(), v.end(), 0);
//std::cout << "call sumOver, res = " << res << std::endl;
//std::cout << "call sumOver, addr: " << &res << std::endl;
}
int main () {
std::vector<int> v { 1,2,3,4,5,6,7,8,9 };
std::vector<int> res(NUM, -1);
std::vector<LockFreeWorkStealingQueue> wsq(4);
{
for (auto i = 0; i < 4; ++i) {
for (auto j = 0; j < NUM / 4; ++j) {
std::cout << "push_back addr: " << &res[i*(NUM/4)+j] << std::endl;
wsq[i].push_back(FunctionWrapper(sumOver, std::ref(v), std::ref(res.at(i*(NUM/4)+j))));
}
}
FunctionWrapper f;
for (auto i = 0; i < 4; ++i) {
for (auto j = 0; j < NUM / 4; ++j) {
if(wsq[i].try_pop_back(f)) {
f();
}
}
}
}
for (auto i = 0; i < 4; ++i) {
for (auto j = 0; j < NUM / 4; ++j) {
std::cout << "res[" << i*(NUM/4)+j << "]=" << res[i*(NUM/4)+j] << std::endl;
}
}
return 0;
}
EDIT: I made a change to functionwrapper.h to refect on the comments. and now it works well.
#include <memory>
#include <functional>
class FunctionWrapper {
private:
std::function<void()> callback;
public:
template <typename F, typename... Args>
FunctionWrapper(F&& f_, Args&&... args_) : callback([f_, args_...]() { f_(args_...); }) {}
void operator()() { callback(); }
FunctionWrapper() = default;
FunctionWrapper(FunctionWrapper&& other) : callback(std::move(other.callback)) {}
FunctionWrapper& operator=(FunctionWrapper&& other) {
callback = std::move(other.callback);
return *this;
}
FunctionWrapper(const FunctionWrapper&) = delete;
FunctionWrapper(FunctionWrapper&) = delete;
FunctionWrapper& operator=(const FunctionWrapper&) = delete;
};
The lambda in FunctionWrapper::Impl
captures references to temporary std::reference_wrapper
instances (produced by std::ref
calls in main
). By the time the lambda is actually called, those temporaries have long been destroyed and the references are dangling. Whereupon your program exhibits undefined behavior, by way of accessing objects whose lifetime has ended.
You want to capture by value instead, as in
Impl(F&& f_, Args&&... args_) {
callBack = [f_, args_...]() { f_(args_...); };
}