GoLang datalogger (bandwidth) memory leak

Question

I'm trying to find the memory leak and I've zeroed it down to this portion of code but I can't find where the memory leak is or how to fix it, when I had some people look into it they suggested it has to do with "tickers" as mentioned here: https://golang.org/src/time/tick.go it "leaks". Any ideas on fixes?

Thanks! :)

package main

import (
    "bufio"
    "encoding/csv"
    "fmt"
    "log"
    "os"
    "time"
)

// Records information about a transfer window: the total amount of data
// transferred in a fixed time period in a particular direction (clientbound or
// serverbound) in a session.
type DataLoggerRecord struct {
    // Number of bytes transferred in this transfer window.
    Bytes uint
}

var DataLoggerRecords = make(chan *DataLoggerRecord, 64)

// The channel returned should be used to send the number of bytes transferred
// whenever a transfer is done.
func measureBandwidth() (bandwidthChan chan uint) {
    bandwidthChan = make(chan uint, 64)
    timer := time.NewTicker(config.DL.FlushInterval * time.Second)

    go func() {
        for _ = range timer.C {
            drainchan(bandwidthChan)
        }
    }()  

    go func() {
        var count uint
        ticker := time.Tick(config.DL.Interval)

        for {
            select {
            case n := <-bandwidthChan:
                count += n

            case <-ticker:
                DataLoggerRecords <- &DataLoggerRecord{
                    Bytes:      count,
                }
                count = 0
            }
        }
    }()

    return bandwidthChan
}

func drainchan(bandwidthChan chan uint) {
    for {
        select {
        case e := <-bandwidthChan:
            fmt.Printf("%s\n", e)
        default:
            return
        }
    }
}


func runDataLogger() {
    f, err := os.OpenFile(dataloc, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
    if err != nil {
        log.Fatalf("[DL] Could not open %s", err.Error())
    }

    bw := bufio.NewWriter(f)
    defer func() {
        bw.Flush()
        f.Close()
    }()

    go func() {
        for {
            time.Sleep(time.Second)
            bw.Flush()
        }
    }()

    w := csv.NewWriter(bw)


    for record := range DataLoggerRecords {
        if record.Bytes != 0 {
            err = w.Write([]string{
                fmt.Sprintf("%d", record.Bytes),
            })
            w.Flush()
        } else {
            continue
        }
    }
    if err != nil {
        if err.Error() != "short write" {
            log.Printf("[DL] Failed to write record: %s", err.Error())
        } else {
            w.Flush()
        }
    }
}

Your code doesn't compile: How to create a Minimal, Complete, and Verifiable example. — peterSO
– peterSO, Commented Mar 9, 2016 at 0:56
Sorry, it is part of larger project, this is just one portion of code that is referenced (hence no main function) — Leonardo J.
– Leonardo J., Commented Mar 9, 2016 at 1:00
I have no idea the context in which this is used, but measureBandwidth creates 2 tickers that are never stopped, and starts 2 goroutines which never return. If you call that repeatedly you'll be "leaking" those resources. — Mr_Pink
– Mr_Pink, Commented Mar 9, 2016 at 2:38
@JimB the idea was that we ran this to do bandwidth logging, it is called via the function gist.github.com/anonymous/b7117ec590aedcad56f8 so yes, they are called frequently; however, how would I go about resolving that then? — Leonardo J.
– Leonardo J., Commented Mar 9, 2016 at 8:14

Community · Accepted Answer · 2017-05-23 10:28:02Z

You are starting 2 time.Ticker and never stopping them, and starting 2 goroutines which never return. Every time you call measureBandwidth you "leak" those resources.

Since you already have a channel which you're only receiving on, you should use that as a signal to return from the counting goroutine. The caller would then close the returned channel to cleanup.

The second goroutine is not needed, and only serves to race the counter to throw away values. The counting goroutine can keep up, and if the send to the logger is too slow, put that in its own select case.

func measureBandwidth() (bwCh chan int) {
    bwCh = make(chan int, 64)

    go func() {
        ticker := time.NewTicker(config.DL.Interval)
        defer ticker.Stop()

        count := 0

        for {
            select {
            case n, ok := <-bwCh:
                if !ok {
                    return
                }
                count += n

            case <-ticker.C:
                DataLoggerRecords <- &DataLoggerRecord{Bytes: count}
                count = 0
            }
        }
    }()

    return bwCh
}

Example: http://play.golang.org/p/RfpBxPlGeW

(minor nitpick, it's usually preferred to use signed types for operating on numeric values like this: see here for one such conversation: When to use unsigned values over signed ones?)

Collectives™ on Stack Overflow

GoLang datalogger (bandwidth) memory leak

1 Answer 1

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related