CODEWITHSUNDEEP
c++c++11
sazr
sazr

Reputation: 25928

Access Violation from Lambda occasionally exhibits itself

I have a lambda that will occasionally cause an access violation when I try to erase an object from a std::vector. I pass a copy of the object to be deleted to the lambda instead of an iterator because the iterator may not point to the correct object come execution time.

Is there a better way to setup a lambda to remove a specific object from a vector? And what is the exact cause of the access violation here? Its difficult to debug as the local and auto variables are null/corrupted when I debug during this error.

... std::vector<CustomLayout> customLayouts; // private class member variable

void App::loadCustomLayouts() {
    CustomLayout customLayout;
    // ... create HWND

    onMessage(WM_RBUTTONDOWN, [customLayout, this]() {

        auto it = std::remove(customLayouts.begin(), customLayouts.end(), customLayout);
        customLayouts.erase(it); // occasionally causes access violation
    });
}

Upvotes: 0

Views: 153

Answers (2)

SergeyA
SergeyA

Reputation: 62583

You are not following the proper remove idiom. It is simple:

vec.erase(std::remove_if(vec.begin(), vec.end(), elem), vec.end());

You do not need to subtract anything or do any custom branches.

Upvotes: 2

Sergey
Sergey

Reputation: 8238

std::remove return the end iterator of the range where all values equal to customLayout were removed. It actually does not remove anything, it just changes elements positions so that "removed" elements are placed after end iterator of the new range. Nevertheless, state of "removed" elements is unspecified and referencing them may cause problems.

Then, in terms of STL end iterator points at the element next to the last element of your range, not the last element itself. That's why any manipulations with end iterator (it in your case) are invalid.

The most probable error scenario is when customLayouts does not contain elements equal to customLayout. Then it points to an element which never existed, and erase attempts to destroy it.

If you want to erase the last element of the new (shorter) range, you should write customLayouts.erase(it - 1);.

Upvotes: 0

Related Questions