Does emptying a std::queue using a scoped queue::swap break any rules?

Question

I have a number of situations where queues are being used, which can grow to sizes in the many hundreds. Unfortunately, there is no one-call-solution to emptying the queue, should it become necessary. I'm wondering if using a scoped queue to do a swap, and then let the scoped queue be destroyed, breaks any memory allocation/management rules?

The following snippet is an example of what I am proposing. Seems to work, unsure of results if used many times over long periods.

#include <cstdlib>
#include <iostream>
#include <queue>
int
main()
{
    std::queue<int> foo;
    foo.push(10);
    foo.push(20);
    foo.push(30);
    std::cout << "size of before foo: " << foo.size() << '\n';

    {
        std::queue<int> bar;
        swap(foo, bar);
    }
    std::cout << "size of after foo: " << foo.size() << '\n';
    return 0;
}

Should be ok: [`std::queue<>::swap()`](https://en.cppreference.com/w/cpp/container/queue/swap). But since you don't specify a different container in the instantiation of `std::queue<>` why don't you simply use a [`std::deque<>`](https://en.cppreference.com/w/cpp/container/deque) directly and [`std::deque::clear()`](https://en.cppreference.com/w/cpp/container/deque/clear)? — Swordfish, May 16 '19 at 21:21
@Swordfish As I understand it, the original author of the code I'm working with wanted to make sure that the queue structure being used forced FIFO. It was a security issue in terms of accreditation. However, his solution to clearing the queue was a loop doing pops until empty. To be consistent I decided to use queue, rather than deque, but was looking for a cleaner way of blowing it away, if necessary. Good question though. — Weedware, May 16 '19 at 21:43

NathanOliver · Answer 1 · 2019-05-16T21:51:50.890

Your code is fine. swap will make foo a default constructed std::queue and when bar is destroyed at the end of the scope it will release the memory that foo was using. Since you aren't using new or delete there is no issue since std::queue "does the right thing" (RAII types are a wonderful thing)

Effectively you've done

std::queue<int>{std::move(foo)}; // move foo into a temporary that is immediately destroyed to release the storage

but your method gives you a stronger guaranteed about the state of foo. Your method leave foo in a default constructed state, while the above method leaves it in a valid, but unspecified state.

Another option is to use one of the solutions provided in Is there a way to access the underlying container of STL container adaptors? to get the underlying container from foo and call clear on it. That would look like

#include <cstdlib>
#include <iostream>
#include <queue>

// function from https://stackoverflow.com/a/29325258/4342498 by jxh: https://stackoverflow.com/users/315052
template <class ADAPTER>
typename ADAPTER::container_type & get_container (ADAPTER &a)
{
    struct hack : ADAPTER {
        static typename ADAPTER::container_type & get (ADAPTER &a) {
            return a.*&hack::c;
        }
    };
    return hack::get(a);
}

int main()
{
    std::queue<int> foo;
    foo.push(10);
    foo.push(20);
    foo.push(30);
    std::cout << "size of before foo: " << foo.size() << '\n';

    get_container(foo).clear();

    std::cout << "size of after foo: " << foo.size() << '\n';
    return 0;
}

score 2 · Accepted Answer · answered May 17 '19 at 01:15

Not only is this completely safe, it's also how move constructors for containers typically work: by swapping with a short-lived (or at least soon-to-be-destroyed) other object then letting the other object die. The destructor then does all the data cleanup for you as quickly as it can. (Here that works around the lack of a clear() member function.)

I reckon if I needed a one-shot "clear" operation, and I really wanted to use a queue (e.g. for forcing FIFO like you've said) then I'd do the same thing.

Although if you can just let the old container go out of scope and switch to working on a newly-declared one then so much the better.

score 1 · Answer 3 · answered May 16 '19 at 22:04

1

Shouldn't break any rules, but you could just do

foo = std::queue<int>{};

answered May 16 '19 at 22:04

evan

1,463
1
10
13

@even I thought about your suggestion at some point on this logic train. I ran it through a test sample in our microprocessor in a loop 1,000,000 times and the size of the memory space for the executable grew substantially--and remained that way. I'm not sure why. But the application is severely space limited and this created its own problem. – Weedware May 16 '19 at 22:15
@Weedware, heap growth due to memory fragmentation is a matter of memory allocation algorithms and allocation patterns. Perhaps you should consider providing a special custom allocator to your container. – Igor G May 16 '19 at 22:37

score 1 · Answer 4 · answered May 16 '19 at 22:32

It's perfectly ok, even if done many times over long period. If it's legal to destroy a non-empty queue and it's legal to swap queues, then it's legal to swap-and-destroy.

And I would say that swapping a queue (or any other container for that matter) to a temporary object is the preferred way of cleaning the container in thread-aware context:

void MyClass::Reset()
{
    // ...
    {
        Container      tmp;
        {
            std::scoped_lock     lock(m_mutex);
            tmp.swap(m_container);
        }
    }
    // ...
}

The reasons being,

Shorter critical section. Since synchronization lock has to be obtained only for the duration of the swap operation which is rather short.
Reduced scope for deadlocks. Since destructors of the contained objects are called after the lock is released. But that, of course, depends on what those destructors can call.

Yakk - Adam Nevraumont · Answer 5 · 2019-05-17T18:44:34.117

1

No need for a scope:

std::queue<int>().swap(foo);

but otherwise, your code is valid.

Here we create a temporary, empty queue. Then we swap its state with the queue with lots of state; so foo has no state, the temporary queue has foo's old state.

Then at the end of the statement, we destroy the temporary queue, along with foo's old state.

There is also a brief one that works with basically every type:

template<class T>
void clear( T& t ) {
  using std::swap;
  swap( static_cast<T&>(T()), t );
}

that isn't plain-old data. Detecting the cases where we'd instead want to zero-initialize T is tricky.

Objects that cannot be default constructed () are going to fail to compile here.

edited May 17 '19 at 18:44

answered May 17 '19 at 01:13

Yakk - Adam Nevraumont

262,606
27
330
524

Can you please provide a one or two liner explaining your answer? I think I understand, but it seems to me that it is swapping a queue into nothing. Intuitively, at least for me, I'm unsure of the technique. – Weedware May 17 '19 at 15:35
@Weedware Yes, you swap your queue into nothing, and all that remains in your queue is ... nothing. Nothing come from nothing, but nothing takes away everything. – Yakk - Adam Nevraumont May 17 '19 at 18:38

Does emptying a std::queue using a scoped queue::swap break any rules?

5 Answers5