# HG changeset patch
# User KUBOTA Yuji <kubota.yuji@lab.ntt.co.jp>
# Date 1485226067 -32400
# Node ID 2836f23c1fbbe7bc2331617f90b30c4eeb3543f3
# Parent  9fb6779d97d122199f3b9646aa9d4321ec2df4fb
Bug 3296: Reference counter should manage TObjectData instance
Reviewed-by: yasuenag
GitHub: https://github.com/HeapStats/heapstats/pull/80

diff -r 9fb6779d97d1 -r 2836f23c1fbb agent/ChangeLog
--- a/agent/ChangeLog	Mon Jan 23 21:50:31 2017 +0900
+++ b/agent/ChangeLog	Tue Jan 24 11:47:47 2017 +0900
@@ -1,3 +1,7 @@
+2017-01-24 KUBOTA Yuji <kubota.yuji@lab.ntt.co.jp>
+
+	* Bug 3296: Reference counter should manage TObjectData instance
+
 2017-01-23 Yasumasa Suenaga  <yasuenag@gmail.com>
 
 	* Bug 3294: Reference counter should be handled in TClassContainer::allClear()
diff -r 9fb6779d97d1 -r 2836f23c1fbb agent/src/classContainer.cpp
--- a/agent/src/classContainer.cpp	Mon Jan 23 21:50:31 2017 +0900
+++ b/agent/src/classContainer.cpp	Tue Jan 24 11:47:47 2017 +0900
@@ -279,6 +279,10 @@
                     && objData->clsLoaderId == expectData->clsLoaderId)) {
                     
                     /* Return existing data on map. */
+                    /*
+                     * We should not increment reference counter because
+                     * we do not add reference.
+                     */
                     existData = expectData;
                 } else {
                     
@@ -300,6 +304,7 @@
             try {
                 /* Append class data. */
                 (*classMap)[klassOop] = objData;
+                atomic_inc(&objData->numRefs, 1);
             } catch(...) {
                 /*
                  * Maybe failed to allocate memory at "std::map::operator[]".
@@ -321,8 +326,10 @@
         /* Broadcast to each local container. */
         for (TLocalClassContainer::iterator it = localContainers.begin();
             it != localContainers.end(); it++) {
-            
-            (*it)->pushNewClass(klassOop, objData);
+            // We should skip myself if "this" ptr is in local container.
+            if (*it != this) {
+                (*it)->pushNewClass(klassOop, objData);
+            }
         }
     }
     /* Release spin lock of containers queue. */
@@ -351,6 +358,7 @@
     
     /* Remove item from map. Please callee has container's lock. */
     classMap->erase(target->klassOop);
+    atomic_inc(&target->numRefs, -1);
     
     /* Get spin lock of containers queue. */
     spinLockWait(&queueLock);
@@ -358,14 +366,17 @@
         /* Broadcast to each local container. */
         for (TLocalClassContainer::iterator it = localContainers.begin();
             it != localContainers.end(); it++) {
-            
-            /* Get local container's spin lock. */
-            spinLockWait(&(*it)->lockval);
-            {
-                (*it)->classMap->erase(target->klassOop);
+            // We should skip myself if "this" ptr is in local container.
+            if (*it != this) {
+                /* Get local container's spin lock. */
+                spinLockWait(&(*it)->lockval);
+                {
+                    (*it)->classMap->erase(target->klassOop);
+                    atomic_inc(&target->numRefs, -1);
+                }
+                /* Release local container's spin lock. */
+                spinLockRelease(&(*it)->lockval);
             }
-            /* Release local container's spin lock. */
-            spinLockRelease(&(*it)->lockval);
         }
     }
     /* Release spin lock of containers queue. */
@@ -404,7 +415,7 @@
             cur != classMap->end(); ++cur) {
             TObjectData *pos = (*cur).second;
             
-            if ((pos != NULL) && (atomic_get(&pos->numRefsFromChildren) == 0)) {
+            if ((pos != NULL) && (atomic_get(&pos->numRefs) == 0)) {
               free(pos->className);
               free(pos);
             }
@@ -1036,7 +1047,7 @@
                 
                 /* If class is prepared remove from class container. */
                 if (unlikely(objData->isRemoved &&
-                            (atomic_get(&objData->numRefsFromChildren) == 0))) {
+                            (atomic_get(&objData->numRefs) == 0))) {
                     
                     /*
                      * If we do removing map item here,
diff -r 9fb6779d97d1 -r 2836f23c1fbb agent/src/snapShotContainer.cpp
--- a/agent/src/snapShotContainer.cpp	Mon Jan 23 21:50:31 2017 +0900
+++ b/agent/src/snapShotContainer.cpp	Tue Jan 24 11:47:47 2017 +0900
@@ -222,7 +222,7 @@
             counter = counter->next;
             
             /* Deallocate TChildClassCounter. */
-            atomic_inc(&aCounter->objData->numRefsFromChildren, -1);
+            atomic_inc(&aCounter->objData->numRefs, -1);
             free(aCounter->counter);
             free(aCounter);
         }
@@ -350,7 +350,7 @@
             : : "r" (newCounter->counter) : "cc", "memory", "%xmm0");
     }
 
-    atomic_inc(&objData->numRefsFromChildren, 1);
+    atomic_inc(&objData->numRefs, 1);
     newCounter->objData = objData;
     
     /* Chain children list. */
@@ -606,7 +606,7 @@
                    * TClassContainer::commitClassChange().
                    */
                   if (objData->isRemoved) {
-                    atomic_inc(&objData->numRefsFromChildren, -1);
+                    atomic_inc(&objData->numRefs, -1);
                     TChildClassCounter *nextCounter = counter->next;
 
                     if (prevCounter == NULL) {
@@ -626,7 +626,7 @@
                                             &childClsData, &parentPrevData,
                                             &parentMorePrevData);
                     if (likely(childClsData != NULL)) {
-                      atomic_inc(&objData->numRefsFromChildren, -1);
+                      atomic_inc(&objData->numRefs, -1);
 
                       if (parentPrevData == NULL) {
                         clsCounter->child = childClsData->next;
diff -r 9fb6779d97d1 -r 2836f23c1fbb agent/src/snapShotContainer.hpp
--- a/agent/src/snapShotContainer.hpp	Mon Jan 23 21:50:31 2017 +0900
+++ b/agent/src/snapShotContainer.hpp	Tue Jan 24 11:47:47 2017 +0900
@@ -86,7 +86,7 @@
     jlong clsLoaderTag;      /*!< Class loader class tag.                    */
     bool isRemoved;          /*!< Class is already unloaded.                 */
     jlong instanceSize;      /*!< Class size if this class is instanceKlass. */
-    int numRefsFromChildren; /*!< Number of references from TChildClassCounter. */
+    int numRefs;             /*!< Number of references.                      */
 } TObjectData;
 
 /*!