[BUG] shared_ptr is not being copied in parallelize_loop

[recsater]

Describe the bug

shared_ptr is not being copied in parallelize_loop.

Minimal working example

      BS::thread_pool pool;

      int x(0), y(1), z(2);
      auto k = std::make_shared<int>(4);

      pool.parallelize_loop(0, 10, [=](const int i, const int j){
        LOG(ERROR) << x << " | " << y << " | " << z << " | " << k;

        }, 2).wait();

Behavior

System information

• CPU model, architecture, # of cores and threads: amd64 20 cores
• Operating system: ubuntu 22.04 x86_64
• Name and version of C++ compiler: g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
• Full command used for compiling, including all compiler flags:
• Thread pool library version: 3.5.0

(Please note that only the latest version of the thread pool library is supported.)

Additional information

used vcpkg port

This is fixed by removing std::forward on line 345 of the parallelize_loop function.

[bshoshany]

Hi @recsater and thanks for opening this issue! Unfortunately, removing the perfect forwarding from parallelize_loop() may introduce other issues. You can allow each loop function to access the shared pointer if you pass it by reference: simply change [=] in the lambda expression to [&] (or [=, &k] if you want to capture the other variables by copy).

[recsater]

Hi @recsater and thanks for opening this issue! Unfortunately, removing the perfect forwarding from parallelize_loop() may introduce other issues. You can allow each loop function to access the shared pointer if you pass it by reference: simply change [=] in the lambda expression to [&] (or [=, &k] if you want to capture the other variables by copy).

I can't use pass by reference because of the lifetime of the shared pointer. What do I do in this case?

[bshoshany]

I think the problem is that the lambda expression itself is a temporary object, since you define it only as a function argument. If you define the lambda expression as a proper object in the same scope as the shared pointer, then the shared pointer will be properly copied into the lambda, and then when you pass the lambda to parallelize_loop(), the use count will be incremented correctly.

Here is a quick example demonstrating that the shared pointer is indeed still owned by the lambda even after both the pointer and the lambda go out of the scope of the main thread:

#include "BS_thread_pool.hpp"

BS::synced_stream sync_out;
BS::thread_pool pool(5);

int main()
{
    BS::multi_future<void> loop_future;
    {
        std::shared_ptr<int> k = std::make_shared<int>();
        *k = 1;
        auto task = [k](int, int)
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(500));
            sync_out.println(k, " -> ", *k);
        };
        loop_future = pool.parallelize_loop(0, 10, task);
    }
    loop_future.wait();
}

Output on my computer:

0000022EA316F1D0 -> 1
0000022EA316F1D0 -> 1
0000022EA316F1D0 -> 1
0000022EA316F1D0 -> 1
0000022EA316F1D0 -> 1

Hope this helps!

[recsater]

thank you so much, bshoshany. I really appreciate it.

[bshoshany]

You're welcome! :)

[bshoshany]

PS: If you like this project, please consider starring it!