Home > Software design >  Problems with map, goroutine and mutex
Problems with map, goroutine and mutex

Time:04-16

This project is made to receive POST routes that will finally count as access to later write to a database. The intuition is to save interaction with the database of another project in production. I decided to do it in go, but I'm new to the language and I'm struggling to understand. I'm trying to make it so that there is no loss or that there are more accesses.

The project basically consists of a controller, a service and two models, just enough to meet the need for which it was created. In my controller I have the function that will be responsible for receiving the POST.

controllers/views.go:

func StoreViews(c *fiber.Ctx) error {
    var songview models.SongView
    err := c.BodyParser(&songview)
    if err != nil {
        return c.Status(403).JSON(fiber.Map{
            "errors": fiber.Map{"request": err.Error()},
        })
    }

    songview.Date = time.Now()

    errs := utils.ValidateStruct(songview)
    if len(errs) > 0 {
        return c.Status(403).JSON(map[string]interface{}{"errors": errs})
    }

    go services.StoreViews(songview)

    return c.SendStatus(fiber.StatusOK)
}

To handle the received data I made these three functions in my service: services/views.go

var (
    StoreViewsMap = make(map[string]*models.SongView)
    StoreControl  sync.RWMutex
)

func StoreViews(sview models.SongView) bool {
    nameKey := strconv.Itoa(int(sview.SongId))   sview.Lang   sview.Date.Format("2006-01-02")
    songview := getSongView(nameKey)
    initSongView(nameKey, songview, sview)
    return true
}

func getSongView(name string) *models.SongView {
    StoreControl.RLock()
    defer StoreControl.RUnlock()
    return StoreViewsMap[name]
}

func initSongView(name string, songview *models.SongView, sview models.SongView) bool {
    StoreControl.Lock()
    defer StoreControl.Unlock()
    if songview == nil {
        insert := models.SongView{
            SongId: sview.SongId,
            Lang:   sview.Lang,
            Date:   sview.Date,
            Views:  0,
        }
        songViewNew := &insert // see if & is needed
        StoreViewsMap[name] = songViewNew
    } else {
        songview.Views = songview.Views   1
    }
    return true
}

I tried to implement RWMutex to get it to do everything without overlapping anything, but it's not working as it should, sometimes it disappears with views, other times it rescues "songview" in the getSongView function wrongly, among several other problems that I found modifying and reviewing my code. The current code is not in the version that I managed to get closer to the expected result, but I didn't save this version so I decided to bring the current code to exemplify what I'm facing.

I would like you to help me understand how I can deal with several concurrent requests disputing, how I can interact with the data in the best possible way and if there is an error in the use of a pointer I am open to understand. To simulate a POST "attack" to my code I'm using this code in another main.go I made for this test.

var limit int = 10

func main() {
    channel := make(chan string)
    for i := 0; i < limit; i   {
        go func(i int) {
            post("http://localhost:3000/views/store", "lang=pt&song_id=296", i)
            channel <- "ok"
        }(i)
        go func(i int) {
            post("http://localhost:3000/views/store", "lang=en&song_id=3016", i)
            channel <- "ok"
        }(i)
        go func(i int) {
            post("http://localhost:3000/views/store", "lang=pt&song_id=3016", i)
            channel <- "ok"
        }(i)
    }
    for i := 0; i < limit*3; i   {
        <-channel
    }
}

func post(url string, json string, index int) {
    payload := strings.NewReader(json)

    client := &http.Client{}
    req, err := http.NewRequest("POST", url, payload)

    if err != nil {
        fmt.Println(err)
        return
    }
    req.Header.Add("Content-Type", "application/x-www-form-urlencoded")

    res, err := client.Do(req)
    if err != nil {
        fmt.Println(err)
        return
    }
    defer res.Body.Close()

    _, err = ioutil.ReadAll(res.Body)
    if err != nil {
        fmt.Println(err)
        return
    }
    if res.StatusCode != 200 {
        fmt.Println(res.StatusCode)
    }
}

My song-view model is this: (I'm just using it to sort the data, although the project is connected to the bank of the project in production, it is read-only)

type SongView struct {
    Id       int64     `json:"id"`
    SongId   int64     `json:"song_id" form:"song_id" gorm:"notNull" validate:"required,number"`
    ArtistId int64     `json:"artist_id"`
    Lang     string    `json:"lang" validate:"required,oneof=pt en es de fr"`
    Date     time.Time `json:"date" gorm:"column:created_at" validate:"required"`
    Views    int64     `json:"views"`
}

CodePudding user response:

I believe that this code can be written a little more easily in Go, but that is not the question. From your description, it appears that the data is lost somewhere. Have you tried the Go data race detector tool? Below is a link https://go.dev/doc/articles/race_detector Can you provide examples of input data where errors/missing items appear?

CodePudding user response:

It happens because your code has a race condition in between read and write to map. Example: G1 - goroutine 1 G2 - goroutine 2

  1. G1: ReadLock and Read songview named "MySong"
  2. G1: MySong doesn't exists. Nil will be inserted and returned.
  3. G1: Unlock
  4. G2: ReadLock and Read songview named "MySong"
  5. G2: MySong exist Nil. Nil will be returned.
  6. G1: WriteLock.
  7. G1: songView = nil, so create a new one. Set counter to 1.
  8. G1: set counter to 1. Insert to map on key "MySong"
  9. G1: Unlock
  10. G2: WriteLock: songView = nil(because you read it on step 2). Create new SongView. Set counter to 1.
  11. G2: unlock

As a result you have 1 "MySong" with counter 1 because you rewrite a previous value. The idea of locking - Atomicity. So, all your operation should be atomic.

func initSongView(name string, sview models.SongView) bool {
    StoreControl.Lock()
    defer StoreControl.Unlock()
    songview := StoreViewsMap[name]
    if songview == nil {
        insert := models.SongView{
            SongId: sview.SongId,
            Lang:   sview.Lang,
            Date:   sview.Date,
            Views:  1, // counter should be 1 because it's a first view
        }
        StoreViewsMap[name] = &insert
    } else {
        songview.Views = songview.Views   1
    }
    return true
}
  • Related