From 0d449029eaee689da88fcf1d0d572188f4e757d6 Mon Sep 17 00:00:00 2001
From: Shark-Teeth <50938330+CtrlSequence@users.noreply.github.com>
Date: Tue, 27 Aug 2019 13:03:41 -0400
Subject: [PATCH] Fix negative RAM usage (#878)

This seems to be causing some issues with clobbering memory values, and
since there is callback functionality already working, this seems
unnecessary.

* Make all calculations local to function

I moved from making the assignments and calculations of certain memory
values to doing the calculations on local variables and assigning them
at the end of the function for update_meminfo().

This is to keep the info.memX variables from having 'intermediary'
values that may give wrong values to other functions if the structure is
read from while the function is currently executing.

As a matter of keeping the variables consistent across function calls, I
removed the zeroing out of certain info struct variables so that if they
are read from, they'll still report a sane value. Since the only change
to the value a direct assignment at the end of the function, they
shouldn't need zeroing out in the first place.
---
 src/linux.cc | 34 ++++++++++++++++++++++++----------
 src/top.cc   |  4 ----
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/src/linux.cc b/src/linux.cc
index 57760fac..c1e0634e 100644
--- a/src/linux.cc
+++ b/src/linux.cc
@@ -179,11 +179,18 @@ int update_meminfo(void) {
   /* unsigned int a; */
   char buf[256];
 
-  unsigned long long shmem = 0, sreclaimable = 0;
+  /* With multi-threading, calculations that require
+   * multple steps to reach a final result can cause havok
+   * if the intermediary calculations are directly assigned to the
+   * information struct (they may be read by other functions in the meantime).
+   * These variables keep the calculations local to the function and finish off
+   * the function by assigning the results to the information struct */
+  unsigned long long shmem = 0, sreclaimable = 0, curmem = 0, curbufmem = 0,
+                     cureasyfree = 0;
 
-  info.mem = info.memwithbuffers = info.memmax = info.memdirty = info.swap =
-      info.swapfree = info.swapmax = info.bufmem = info.buffers = info.cached =
-          info.memfree = info.memeasyfree = 0;
+  info.memmax = info.memdirty = info.swap = info.swapfree = info.swapmax =
+      info.memwithbuffers = info.buffers = info.cached = info.memfree =
+          info.memeasyfree = 0;
 
   if (!(meminfo_fp = open_file("/proc/meminfo", &reported))) { return 0; }
 
@@ -211,8 +218,8 @@ int update_meminfo(void) {
     }
   }
 
-  info.mem = info.memwithbuffers = info.memmax - info.memfree;
-  info.memeasyfree = info.memfree;
+  curmem = info.memwithbuffers = info.memmax - info.memfree;
+  cureasyfree = info.memfree;
   info.swap = info.swapmax - info.swapfree;
 
   /* Reclaimable memory: does not include shared memory, which is part of cached
@@ -220,17 +227,24 @@ int update_meminfo(void) {
      Note: when shared memory is swapped out, shmem decreases and swapfree
      decreases - we want this.
   */
-  info.bufmem = (info.cached - shmem) + info.buffers + sreclaimable;
+  curbufmem = (info.cached - shmem) + info.buffers + sreclaimable;
 
-  /* Now (info.mem - info.bufmem) is the *really used* (aka unreclaimable)
+  /* Now ('info.mem' - 'info.bufmem') is the *really used* (aka unreclaimable)
      memory. When this value reaches the size of the physical RAM, and swap is
      full or non-present, OOM happens. Therefore this is the value users want to
      monitor, regarding their RAM.
   */
   if (no_buffers.get(*state)) {
-    info.mem -= info.bufmem;
-    info.memeasyfree += info.bufmem;
+    curmem -= curbufmem;
+    cureasyfree += curbufmem;
   }
+
+  /* Now that we know that every calculation is finished we can wrap up
+   * by assigning the values to the information structure */
+  info.mem = curmem;
+  info.bufmem = curbufmem;
+  info.memeasyfree = cureasyfree;
+
   fclose(meminfo_fp);
   return 0;
 }
diff --git a/src/top.cc b/src/top.cc
index d3c5a7ad..a80e3b7a 100644
--- a/src/top.cc
+++ b/src/top.cc
@@ -390,10 +390,6 @@ static void process_find_top(struct process **cpu, struct process **mem,
 }
 
 int update_top() {
-  // XXX: this was a separate callback. and it should be again, as soon as it's
-  // possible
-  update_meminfo();
-
   process_find_top(info.cpu, info.memu, info.time
 #ifdef BUILD_IOSTATS
                    ,