Fix CI.

Figuring out this memory leak was an epic story. I was able to quickly
track down that it was caused by commit 3365, in particular the
overloading of to-text to support characters. But beyond that I was
stumped. Why were layer 3's trace_stream::curr_stream objects being
leaked in layer 81 by a change in layer 59?!

Triaging through the layers, I found the following:

  layer 81 - leaks 2 blocks (in clear-line-erases-printed-characters)
  layer 83 - leaks 4 additional blocks (in clear-line-erases-printed-characters-2)

I also figured out that the leaks were happening because of the call to
'trace' on a character inside print:character (that's the 'print'
function called with a character argument)

  trace 90, [print-character], c

So I tried to create a simple scenario:

  scenario trace-on-character-leaks [
    1:character <- copy 111/o
    trace 90, [print-character], 1:character
  ]

But that triggered no leaks. Which made sense because there were plenty
of calls to that 'trace' instruction in print:character. The leak only
happened when print:character was called from clear-line. Oh, it happens
only when tracing 0/nul characters. Tracing a Mu string with a nul
character creates an empty C++ string, which is weird. But why should it
leak memory?!

Anyway, I tried a new scenario at layer 62 (when 'trace' starts
auto-converting characters to text)

  scenario stashing-nul-character-leaks [
    1:character <- copy 0/nul
    trace 90, [dbg], 1:character
  ]

But still, no leak! I played around with running layers until 70, 80.
But then it didn't leak even at layer 82 where I'd seen it leak before.
What had I done?

Turns out it was only leaking if I used names for variables and not
numeric addresses. Eventually I was able to get layer 59 to leak:

  scenario stashing-nul-character-leaks [
    c:character <- copy 0/nul
    x:text <- to-text c
    trace 90, [dbg], x
  ]

At that point I finally went to look at layer 3 (I'd been thinking about
it before, but hadn't bothered to *actually go look*!) And the leak was
obvious.

In the end, all the information I needed was right there in the leak
report. The reason this was hard to find was that I wasn't ready to
believe there could be a bug in layer 3 after all these months. I had to
go through the five stages of grief before I was ready for that
realization.

Final mystery: why was the memory leak not triggered by numeric
variables? Because the transform to auto-convert ingredients to text
only operated on named variables. Manually performing the transform did
leak:

  scenario stashing-text-containing-nul-character-leaks [
    1:text <- new character:type, 1/capacity
    put-index *1:text, 0, 0/nul
    trace 90, [dbg], 1:text
  ]
This commit is contained in:
Kartik K. Agaram 2016-09-16 14:37:52 -07:00
parent 02abf5d114
commit aa2fd725c0
1 changed files with 5 additions and 4 deletions

View File

@ -133,10 +133,11 @@ struct trace_stream {
void trace_stream::newline() {
if (!curr_stream) return;
string curr_contents = curr_stream->str();
if (curr_contents.empty()) return;
past_lines.push_back(trace_line(curr_depth, trim(curr_label), curr_contents)); // preserve indent in contents
if (!Hide_errors && curr_label == "error")
cerr << curr_label << ": " << curr_contents << '\n';
if (!curr_contents.empty()) {
past_lines.push_back(trace_line(curr_depth, trim(curr_label), curr_contents)); // preserve indent in contents
if (!Hide_errors && curr_label == "error")
cerr << curr_label << ": " << curr_contents << '\n';
}
delete curr_stream;
curr_stream = NULL;
curr_label.clear();