Cannot access vector (segmentation fault) after pop_back() from it

Question

I am writing a simple in-memory database. It should support transaction, meaning that BEGIN command creates a new block, the operations in which can be canceled by ROLLBACK command.

I am implementing the list of transaction blocks using a vector. In case of BEGIN a new block is created by push_back()ing an unordered_set of all keys in that transaction block. This is defined as vector<unordered_set<string>> key_in_blocks

What follows is an example of the commands. SET sets value to a variable, GET gets the value of the variable.

BEGIN
SET a 10
GET a       <-- this prints 10
BEGIN
SET a 20
GET a       <-- this prints 20
ROLLBACK
GET a       <-- this prints 10
ROLLBACK    <-- Segmentation Fault here!

So there is a default block at first, and keys_in_block would look like [()], here () is denoting set, [] denotes vector.

When BEGIN is issued, a new block is created, and SET adds a key a to the block, so keys_in_block = [(), ('a')].

The next BEGIN & SET part is similar, making keys_in_block look like [(), ('a'), ('a')].

Now ROLLBACK undoes operations done in the last block, and keys_in_block should be [(), ('a')], because it is pop_back()ed.

I think my program does what it needs to do up to this point.

On the second ROLLBACK, it throws segmentation fault, and it looks like I cannot even access the keys_in_block variable at all.

Below is my code snippet. It runs inside an infinite while loop, taking commands from user.

} else if (command == "BEGIN") {
    int num_blocks = keys_in_block.size();
    keys_in_block.resize(num_blocks + 1); //new transaction block
} else if (command == "ROLLBACK") {
    if (keys_in_block.size() <= 1) {
        cout << "NO TRANSACTION" << endl;
    }
    else {
        auto &recent_keys = keys_in_block.back();
        // undo operations for keys in the most recent transaction block
        for (const string &key : recent_keys) {
            //...
            //...    a bunch of operations that undoes what's done
            //...    in the most recent transaction block.
            recent_keys.erase(key); // erase all keys in the last entry in `keys_in_block`.
        }
        auto & aa = keys_in_block; //***Just for testing, not relevant to the program.
                                   //This throws Segmentation Fault on second ROLLBACK.
        keys_in_block.pop_back();  //Pop back the last transaction block.
    }
}

In the code snippet, I marked where the segmentation fault is thrown using //***.

I added that line to because keys_in_block.pop_back() threw segmentation fault, and wanted to see if it throws seg fault just by accessing it.

To me, the algorithm looked absolutely correct, and I couldn't find out what the cause of the problem was.

ROLLBACK doesn't execute when keys_in_block has one block or less, so pop_back() cannot be an issue.

If you need the code for the SET command part, let me know, but I don't think there is a problem in that code.

[Erasing from a std::vector while doing a for each?](http://stackoverflow.com/q/3938838/86967) — Brent Bradburn, Sep 16 '15 at 02:45
[Minimal complete example](http://stackoverflow.com/help/mcve)? — Beta, Sep 16 '15 at 02:47
@nobar I can't believe it!! I commented out `recent_keys.erase(key);` because that wasn't necessary, and it worked!! and yet, I cannot figure out why. — Eric, Sep 16 '15 at 02:49
This is weird because `recent_keys` is just a reference to the last element in the vector, and I was removing elements in that last element in the vector using `erase`. Can you explain what's happening, and how I could avoid making the same mistake? — Eric, Sep 16 '15 at 02:52
You deleted everything from your `std::vector` then tried to pop the back. Undefined behavior. — erip, Sep 16 '15 at 02:52
Well, iterating and modifying the container at the same time of course blows it up, since the iterator would go invalid — Marson Mao, Sep 16 '15 at 02:56
@immibis Yes, I inserted `cout`s before and after that line, and the `cout` before it prints, whereas one after doesn't, and what nobar suggested fixed it. I just want to figure out what is happening. — Eric, Sep 16 '15 at 03:07

score 1 · Accepted Answer · answered Aug 12 '16 at 08:42

I commented out recent_keys.erase(key); because that wasn't necessary, and it worked!! and yet, I cannot figure out why.

This is the reason that your program crashes, since you are deleting the element while iterating the container at the same time. The for (const string &key : recent_keys) loop is trying to running from the begin to the end, however you remove the element in the loop, so that the iterator goes invalid, so the loop can't continue and crashes when it tries to increment the iterator.

This is weird because recent_keys is just a reference to the last element in the vector, and I was removing elements in that last element in the vector using erase. Can you explain what's happening, and how I could avoid making the same mistake?

Yes recent_keys is a reference to the last element of the vector, but you should not remove element while iterating it. If you want to avoid making the same mistake then never do anything like this again.

In your example you can just iterate the recent_keys, then let keys_in_block.pop_back() do the clean up. For other cases, you might want to iterate the container and collect element references, then remove those elements after done iterating the container, you can use vector's erase combined with std::remove_if to accomplish such thing.

Cannot access vector (segmentation fault) after pop_back() from it

1 Answers1