Multithreading MemoryAccess Error

[native briar]

I am trying to multithread multiple function calls using this function:

template <typename Range, typename Func>
void parallel_for_each(ThreadPool& tp, Range& range, Func func) {
    if (range.size() > 1) {
        auto len = range.size();
        auto num_threads = std::min((size_t)THREAD_AMOUNT, len);
        auto step = len / num_threads;

        for (size_t i = 0; i < len; i += step)
            tp.enqueue([i, step, func, &range] {
                auto curr = std::next(std::begin(range), i);
                auto end = std::next(curr, step);
                for (; curr != end; ++curr)
                    func(*curr);
            });
    }
}

That I am using like so:

//Check tank collision and nudge tanks away from each other
{
    std::lock_guard<std::mutex> lock(tanks_mutex);
    parallel_for_each(*thread_pool, tanks, [this](Tank& tank) { return check_collision(tank); });
}
    
//Update tanks
{
    std::lock_guard<std::mutex> lock(tanks_mutex);
    parallel_for_each(*thread_pool, tanks, [this](Tank& tank) { return update_tank(tank); });
}

//Update smoke plumes
{
    std::lock_guard<std::mutex> lock(smokes_mutex);
    parallel_for_each(*thread_pool, smokes, [this](Smoke& smoke) { return update_smoke_plume(smoke); });
}
etc...

But I get:

Exception thrown at 0x0000000140001A2F in Tmpl8_2019-01_debug.exe: 0xE0736171: Access violation reading location 0xFFFFFFFFFFFFFFFF.
Exception thrown at 0x00007FFB085F051C (KernelBase.dll) in Tmpl8_2019-01_debug.exe: 0xC0000005: Access violation reading location 0xFFFFFFFFFFFFFFFF.

[hot furnace]

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question run !howto ask.

[native briar]

How can I start debugging this?

Is there anything wrong with my parallel_for_each function?

Essentially just wanted a good way of multithreading a large number of function calls on a vector of instances of tank but it's not easy

[honest trout]

Are tanks or smokes alive until all of the operations complete? cause you are passing them by reference.

Also, you are locking tanks_mutex and smokes_mutex which I assume are being used to prevent some access to tanks and smokes but it doesn't really make sense becasue 1. you are passing them for simultaneous access from multiple threads and 2. the lock guard will be unlocked as soon as you call parrellel_for_each anyways.

And this is not really related to multithreading but you should probably make the parameters of parallel_for_each forwarding references to make the function more general purpose

Btw, you only need to see which line caused the exception to debug the problem... (the line your IDE points to when you run your code)

I missed one thing. Maybe it's not tanks or smokes but this that you are capturing in the lambda for parallel_for_each that is not alive. Since you are calling its member functions, this should be alive until all the operations complete.

[native briar]

Yes

[native briar]

I didn't know it worked like that. I hoped it would lock tanks while it called check_collision on all its members, then unlocking and allowing update_tank to be called on all of them etc

[native briar]

this should be live, as it is the instance of the Game class which exists until the simulation is over

[native briar]

I dont know if this is useful or not

thank you for your help btw, ive been stuck at this for a while

in the console it freezes after this, and when i close the program i get something like Access violation reading location <address>. lots of times

[hot furnace]

@native briar Has your question been resolved? If so, run !solved :)

[honest trout]

#include <iterator>
#include <vector>
#include <iostream>

int main()
{
    std::vector<int> arr{ 1, 2, 3 };
    auto iteratorBeyondEnd = std::next(arr.begin(), 6);

    if (iteratorBeyondEnd != arr.end())
    {
        std::cout << *iteratorBeyondEnd << std::endl;
    }
}

I think this what's happening. In your code when you do auto end = std::next(curr, step); it is possible that the iterator goes beyond end. I got the same exception as yours by running the code above.

[honest trout]

Maybe what you are looking for is not a lock but a waiting mechanism like std::condition_variable? std::promise and std::future are also available if you want something a bit more higher level.

[native briar]

I see, I will try to fix that

if that is in fact happening

curr > end always by 32

that's supposed to be the other way around?

[native briar]

idk man I am kinda done with this i'll just use threadpools in the code itself everywhere

like this is almost literally impossible to debug for me

the errors are completely useless and I dont know what to do

[fast cedar]

are you the author of this ThreadPool?

or is this someone else's code

[native briar]

No the threadpool was provided, i am supposed to optimize this code for a school assignment

thought the best way would be to just make one function instead of repeat the code everywhere

[fast cedar]

you need to build in debug mode and set compiler optimizations to zero

so that you can see this error message's backtrace

your lambdas are using pointers to things which dissapear asynchronously I assume, but that pointer is no longer valid, so you get use after free bugs

which the AddressSanitizer caught

[native briar]

It is

and I was building in debug

where do I look

do I stop debugging and then look in console?

[fast cedar]

run the program without a debugger and look at the console output

[native briar]

because if so I dont see what I can do with this

[fast cedar]

that's not the console output

that is the build log

can you upload your files

[native briar]

Is this it

application started.
X cell amount : 45
Y cell amount : 23
Threads available: 16
=================================================================
==15860==ERROR: AddressSanitizer: attempting double-free on 0x103380d4fc80 in thread T25:
==15860==WARNING: Failed to use and restart external symbolizer!

D:\Programming\School\periode-2---optimalisatie-pepijn-s\x64\Tmpl8_2019-01_debug.exe (process 15860) exited with code -1.
To automatically close the console when debugging stops, enable Tools->Options->Debugging->Automatically close the console when debugging stops.
Press any key to close this window . .

That's all that is in the console

[fast cedar]

why does windows refuse to print the backtrace in the console wtf

[native briar]

yes let me try, it's a big codebase that we got that we a re supposed to optimize

[fast cedar]

you are not compiling with the correct flags iwith visual studio

!asan

[hot furnace]

How To Use Sanitizers

Sanitizers are tools which generate additional code in your program that can catch many common programming mistakes, such as:

• accessing arrays out of bounds
• signed integer overflows
• race conditions

General Advice

Not all sanitizers can be combined, but when they can, use e.g.: -fsanitize=address,undefined to combine them. Always compile with debug info to get line numbers, variable names, etc.

GCC 4.8+

• -fsanitize=address
• -fsanitize=undefined
• -fsanitize=thread
• -g for debug info

clang 3.1+

• -fsanitize=address
• -fsanitize=undefined
• -fsanitize=thread
• -fsanitize=memory
• -g for debug info

MSVC 19.27+ and VS 2019 16.9+

• -fsanitize=address
• -Zi for debug info

Sample Program

int main(void) {
    int x;
    return x;
} ```

`-fsanitize=memory -g` Output

SUMMARY: MemorySanitizer: use-of-uninitialized-value /tmp/test.cpp:3:5 in main > Exiting (3:5 is line and column of return)

Note: The sanitizer lists for GCC and clang are not exhaustive

[fast cedar]

use -Zi

[native briar]

I did that sanitizer

Let me recheck though

I had this already

[fast cedar]

add -Zi

[native briar]

With visual studio what I have there is the only available option

[fast cedar]

noooo, leave that field to "Yes"

there is a separate setting in Visual Studio

for passing -Zi

[native briar]

this

was already on

?

Is that what you mean

[fast cedar]

aha

I want to run your program, can you send your files

[native briar]

yes

[fast cedar]

it will save me hours

[native briar]

It's uploading, 33 mb for some reason

[fast cedar]

I only need your source files, not the entire project folder

those are tiny

[native briar]

Oh

alright

[fast cedar]

I still have no clue why windows refuses to dump the expected verbose output of Asan, very strange

[native briar]

I hope thats all

I removed x64 and lib folder

the parallel_for_each function is in thread_pool.h

and I am using that at line 170 onward in game.cpp

the program crashes at frame 4, so it goes through the update method 3 times before it actuall goes wrong

[fast cedar]

you are compiling GL and SDL

I don't have these

I can't help you

[native briar]

oh

alright

thanks anyway

[hot furnace]

@native briar Has your question been resolved? If so, run !solved :)

[honest trout]

Did you print memory addresses of curr and end? The values don't really make sense.
Essentially, your range.size() is 3875 so say your step is 4. Then your i will be 0, 4, ..., 3872. When i reaches 3872, your curr points to the element in the vector with the index i which is 3872, and your end points to the element with the index 3876, but this is beyond the length of the vector. So your program crashes when you do func(*curr) (when curr advances enough to go beyond the very end of the vector).

[honest trout]

Not completely useless, because it literally said "cannot seek vector iterator after end" in the photo you posted

[native briar]

That's true but the errors are different each time

let me check again about the curr, end step and i values

            for (size_t i = 0; i < len; i += step)
                tp.enqueue([i, step, func, &range] {
                    auto curr = std::next(std::begin(range), i);
                    auto end = std::next(curr, step);
                    printf("curr: %d, end: %d, i: %d, range.size(): %d, step: %d\n", *curr, *end, i, range.size(), step);
                    
                    for (; curr != end; ++curr)
                        func(*curr);
                });

I am not printing curr and end correctly

but i and step are correct

[honest trout]

Is your range.size() different each time? Earlier the value was 3875 which is not divisible by 256. In that case it will crash. This time, probably not.

[native briar]

It should be 4096 and get smaller overtime

In the screenshot the range was that of tanks, which includes tanks blue and tanks red

as far as I see it is working correctly, i gets incremented by step and a task is added to the threadpool

I can step through the first 3 frames fine

I think this is where I get the error

AS you can see len is 887, but every time I stepped the range increased in size and not equal to the len anymore

so that must mean the lock isn't working right? Like kolio suspected

[honest trout]

your function will never work with a vector of changing length

[native briar]

yes

do you know how I should lock the vector then?

[honest trout]

std:condition_variable or some other things. I refer you to C++ Concurrency in Action.

And locking is not really for that purpose. You really need to study the basics of concurrency starting from the basics with the basic in mind.

[hot furnace]

This question thread is being automatically closed. If your question is not answered feel free to bump the post or re-ask. Take a look at !howto ask for tips on improving your question.