Call reset(nullptr) in scoped_ptr's.

base/memory/scoped_ptr_unittest.cc

Index: base/memory/scoped_ptr_unittest.cc
diff --git a/base/memory/scoped_ptr_unittest.cc b/base/memory/scoped_ptr_unittest.cc
index 71d995c452e826ad1faeb28f315433077f338e88..6fe99dd077ed017c2c1204617739fa0358bb516e 100644
--- a/base/memory/scoped_ptr_unittest.cc
+++ b/base/memory/scoped_ptr_unittest.cc
@@ -715,4 +715,14 @@ TEST(ScopedPtrTest, ReferenceCycle) {
   // deletes the underlying pointer. This behaviour is consistent with the
   // definition of unique_ptr::reset in C++11.
   a->b.reset();
+
+  // Go again, but this time, break the cycle by invoking |a|'s destructor. This
+  // tests that the implementation of ~scoped_ptr doesn't infinitely recurse
+  // into the destructors of |a| and |a.b|. Note, deleting |a| instead will

[Peter Kasting, 2015/09/23 20:35:49]

Nit: a.b -> a->b

[Anand Mistry (off Chromium), 2015/09/23 23:06:34]

Done.

+  // cause |a| to be double-free'd because |a.b| owns |a| and deletes it via its
+  // destructor.

[Peter Kasting, 2015/09/23 20:35:49]

Nit: Maybe change this last sentence to the following, assuming it's
accurate/you agree:

Note that unless code actually calls a destructor directly like this, the only
way to trigger this call chain normally would be to delete one of the pointers,
which would cause a double-free.  Still, a double-free (and then hopefully
crash) is better than having the program hang.

[danakj, 2015/09/23 20:38:10]

I think I prefer the original for a unit test comment.

[Anand Mistry (off Chromium), 2015/09/23 23:06:34]

danakj: Do you mean what I wrote, or what pkasting suggest?

+  a = new StructA;
+  a->b.reset(new StructB);
+  a->b->a.reset(a);
+  a->~StructA();
 }