2382. Unclear order of container update versus object destruction on removing an object

Section: 16.4.6.9 [reentrancy] Status: Pending NAD Submitter: Peter Kasting Opened: 2014-05-06 Last modified: 2014-11-04 10:26:50 UTC

Priority: 2

View all other issues in [reentrancy].

View all issues with Pending NAD status.

Discussion:

The standard does not seem to discuss reentrant access to a container during removal of an element, leaving it unclear whether a removed object is destroyed before or after it is removed from the container. For example, the behavior of the following code seems to be unspecified:

#include <iostream>
#include <map>
#include <memory>

struct T;
typedef std::map<int, std::shared_ptr<T>> TMap;

struct T {
  T(TMap* t_map, int index) : t_map(t_map), index(index) {}
  ~T() {
    std::cout << "Object " << index << " is ";
    if (t_map->count(index))
      std::cout << "destroyed before being removed from the map" << std::endl;
    else
      std::cout << "removed from the map before being destroyed" << std::endl;
  }

  static void AddToMap(TMap* map, int index) {
    (*map)[index] = std::make_shared<T>(map, index);
  }

  TMap* t_map;
  int index;
};

int main()
{
  TMap t_map;
  T::AddToMap(&t_map, 0);
  T::AddToMap(&t_map, 1);
  t_map.erase(1);
  t_map.erase(0);
}

The output of this program in Visual Studio 2013 is:

Object 1 is removed from the map before being destroyed
Object 0 is destroyed before being removed from the map

The core issue here is whether an object removed from a container should be destroyed before or after it is removed from the container. The current standard seems to be silent on this issue. The above output demonstrates that the behavior is actually inconsistent. (It's difficult to fully describe Visual Studio's behavior; for example, changing main() in the above example to the following:)

int main()
{
  TMap t_map;
  T::AddToMap(&t_map, 0);
  T::AddToMap(&t_map, 1);
  T::AddToMap(&t_map, 2);
  T::AddToMap(&t_map, 3);
  t_map.erase(3);
  t_map.clear();
}

(...gives this output:)

Object 3 is removed from the map before being destroyed
Object 2 is destroyed before being removed from the map
Object 1 is destroyed before being removed from the map
Object 0 is removed from the map before being destroyed

In my opinion, the standard should explicitly describe when objects are destroyed as part of removal from a container. To me, it makes the most sense to say that objects should be removed from the container before they are destroyed.

[2014-05-07, Jeffrey Yasskin comments]

I think there are two main points here beyond this writeup:

  1. We can't make recursive use of a standard library container valid in all cases.

  2. If recursion through especially erase() is undefined behavior, that's pretty scary for existing large applications with code in destructors. Of course, "scary" doesn't mean we have to define the behavior.

I'll add a third: The language in 16.4.6.9 [reentrancy] nearly makes this undefined behavior already. I think any fix is probably going to live there, and extend the current "implementation-defined" on recursive reentrancy for individual functions to recursive reentrancy on class instances. I'm not sure exactly how to word that.

[2014-06 Rapperswil]

STL: We need more wording about how container methods can be reentrency.

Jeffrey: The title for this issue is confusing, what we really want is "reentrancy for objects".

Alisdair: Should we then close 2382 as NAD with a link to the new issue?

Proposed resolution: