Search code examples
c++11templatesc++14moveperfect-forwarding

Why a member function in a class template creates the same object in the same address


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 :(

  • push_back addr: 0x21d3ea0 bk: 0 &data: 0x7ffe36802380
  • push_back addr: 0x21d3ea4 bk: 0 &data: 0x7ffe36802380
  • push_back addr: 0x21d3ea8 bk: 0 &data: 0x7ffe36802380
  • push_back addr: 0x21d3eac bk: 0 &data: 0x7ffe36802380
  • res[0]=-1
  • res[1]=-1
  • res[2]=-1
  • res[3]=45

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;
};

Solution

  • 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_...); };
    }
    

    Demo