Improving safe global variables with generic or inheritance

[Zhaoxun Yan]

Hi gophers! In the last post I proposed a way to avoid I/O race on any global variable:

 https://groups.google.com/g/golang-nuts/c/4AW1Ss9Tjp0

However, the code does not seem tidy, since each type needs to repeat defining the two methods Save and Load, though using this methods is easy.

So I did a little play around, but got stuck if I want to add in new methods to specific global types, such as tick to all integer types, or push to all slice types:

-------------------------------------------------------------

package main

import (
    "fmt"
    "sync"
)

type Global struct {
    mutex sync.Mutex
    value any
}

func (g *Global) Save(v any) {
    g.mutex.Lock()
    g.value = v
    g.mutex.Unlock()
}

func (g *Global) Load() any {
    g.mutex.Lock()
    defer g.mutex.Unlock()
    return g.value
}

var S = &Global{}

func main() {

    S.Save(5)
    fmt.Printf("S=%v, S.mutex=%v, S.value=%d \n", S, S.mutex, S.value)
    fmt.Printf("S=%v, S type as %[1]T", S.Load())
}

//New global types that has methods beyond Save and Load:
/*
type countable interface {
    ~int | ~int8 | ~int16 | ~int32 | ~int64
}

type Counter struct {
    mutex sync.Mutex
    value countable
}

func (c *Counter) Tick() {
    c.mutex.Lock()
    c.value += 1
    c.mutex.Unlock()
}
*/

=====================================

It worked well with the "Global" struct type.

However, as I want to narrow the scope of this type down to generate integer types (as in the commented code), it encountered two obstacles:

1) It is not legal to embed a generic inside a struct, nor can it make generalized computation  =+1

2) Inheritance is not available in golang, so type "Counter" cannot inherit type "Global" and get its methods automatically. I need to repeat Save and Load methods to "Counter".

Am I correct? Or can you improve it?

[Axel Wagner]

Just to be clear, are you aware of the sync/atomic package?

https://pkg.go.dev/sync/atomic

There are also some changes in there for Go 1.18, specifically the addition of some types, so that only atomic operations can be done:

https://pkg.go.dev/sync/atomic@master

I mention this because atomic.Value and atomic.Pointer[T] are essentially what you are suggesting here.

On Mon, May 23, 2022 at 8:04 AM Zhaoxun Yan <yan.z...@gmail.com> wrote:

However, as I want to narrow the scope of this type down to generate integer types (as in the commented code), it encountered two obstacles:

1) It is not legal to embed a generic inside a struct, nor can it make generalized computation  =+1

2) Inheritance is not available in golang, so type "Counter" cannot inherit type "Global" and get its methods automatically. I need to repeat Save and Load methods to "Counter".

Am I correct? Or can you improve it?

You are correct that there is no way in Go to write a type which gets all methods from an embedded field using generics. However, that is a good thing. For example, say you could write

type Locked[T any] struct {

    sync.Mutex

    T

}

 

And this would get the methods of T and the methods of sync.Mutex. Then a user could do

type Counter int64

func (c *Counter) Increment() { *c += 1 }

func main() {

    var c Locked[Counter]

    go c.Increment()

    go c.Increment()

}

And get a data race. That is, a method can modify a value in a way that is incompatible with what your wrapper type is trying to do.

That's really the crux of both of the obstacles you mention. You can't run arbitrary computations and you can't promote methods, because *not all computation and not all methods can be made concurrency safe this way*.

It's best to be intentional about it and explicitly acquire and release a mutex around the critical sections of your code - because ultimately, only your code knows which sections are critical.

 

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/baa74bff-6688-4d39-843b-c99a4fea2d1an%40googlegroups.com.

[Brian Candler]

> It's best to be intentional about it and explicitly acquire and release a mutex around the critical sections of your code - because ultimately, only your code knows which sections are critical.

That's really essential.  For example, using your library, the following code is most definitely *not* race-free:

tmp := v.Load()

tmp = tmp + 1

v.Save(tmp)

The mutex has to protect the entire sequence, not the individual load and save operations.

[Axel Wagner]

FWIW a compromise is to have

type Locked[T any] struct {

    mu sync.Mutex

    val T

}

func (l *Locked[T]) Do(f func(T) T) {

    l.mu.Lock()

    defer l.mu.Unlock()

    l.val = f(l.val)

}

This forces modifications to be under the protection of a mutex while also allowing those modifications to do arbitrary things.

The downside is that this might do more locking than is strictly needed. So, under contention, it might perform significantly worse than "manually" managing the critical section.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/50d6ab3d-46d0-4357-b5fa-d99b73ef2adan%40googlegroups.com.

[Zhaoxun Yan]

There are also some changes in there for Go 1.18, specifically the addition of some types, so that only atomic operations can be done:

https://pkg.go.dev/sync/atomic@master

Yes but the syntax is confusing, e.g. the code below fails:

package main

import (
    "fmt"

    "sync/atomic"
    )
   
var a atomic.Int32

func main() {
    fmt.Println(&a.Load())   
}

although the manual says:

type Int32 ¶

type Int32 struct {
	// contains filtered or unexported fields
}

An Int32 is an atomic int32. The zero value is zero.

func (*Int32) Load ¶

func (x *Int32) Load() int32

Load atomically loads and returns the value stored in x.

That's really essential.  For example, using your library, the following code is most definitely *not* race-free:

tmp := v.Load()

tmp = tmp + 1

v.Save(tmp)

Of course not. Neither can misuse of atomic or sync.Map prevent race.

I have already shown how simple `=` and `==` caused race on reading occasions in the last post.

Strict prevention should have been enforced at the syntax level just like GIL in Python or ownership in Rust,

and adding a patch cannot fix loop-holes or side-attacks whatsoever.

[Axel Wagner]

On Mon, May 23, 2022 at 10:19 AM Zhaoxun Yan <yan.z...@gmail.com> wrote:

Yes but the syntax is confusing, e.g. the code below fails:

That's because you can't take the address of a call expression. That is, your code is interpreted as &(a.Load()), while you might have intended (&a).Load(), which you can also just write as a.Load(). 

That's not really about the API, it's how the language works. Note that with your original `Global` type, you get exactly the same error if you write &S.Load().

[Zhaoxun Yan]

type Locked[T any] struct {

    mu sync.Mutex

    val T

}

func (l *Locked[T]) Do(f func(T) T) {

    l.mu.Lock()

    defer l.mu.Unlock()

    l.val = f(l.val)

}

Could you elaborate how 'Save/Store' , 'Load' and 'Tick / Increase '  get applied to the code above?

Thanks.

You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/3javezRHm98/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAEkBMfG63G2%2BCk5VVmAhT1UMxpVsxy88C4gOPH%3DxV%3DZnXXUjmA%40mail.gmail.com.

[Axel Wagner]

You could add them as methods. But as I said, I don't think they are good APIs.

I think the `Do` I described is the most reasonable compromise, if you really want a one-size-fits-all generic solution.

Otherwise, use fine-grained locking or sync/atomic specific to your use-case.

[Zhaoxun Yan]

`&` won't make any difference:

undefined: atomic.Int32

[Axel Wagner]

It would be helpful if you could link to actual code, preferably on the playground.

That error message seems to indicate that you are not using gotip.

I got mixed up earlier, when I wrote Go 1.18. I meant Go 1.19, the next Go version.

[Zhaoxun Yan]

---------- Forwarded message ---------
From: Zhaoxun Yan <yan.z...@gmail.com>
Date: Mon, May 23, 2022 at 6:05 PM
Subject: Re: [go-nuts] Improving safe global variables with generic or inheritance
To: Brian Candler <b.ca...@pobox.com>

That's really essential.  For example, using your library, the following code is most definitely *not* race-free:

tmp := v.Load()

tmp = tmp + 1

v.Save(tmp)

It is race free as long as `tmp` is a local variable.

Although you may argue that another goroutine may have already altered v, that is an atomicity issue but won't cause a `panic` or crash. My resolution is to add a "Tick/Increase" method to substitute the above procedure.

Also the `value` entry in the struct is in small cases, prohibiting getting accessed when imported, so the side-attack to directly call `v.value` won't pass the compile phase, which is much safer than the conventional `atomic` package.

You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/3javezRHm98/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/50d6ab3d-46d0-4357-b5fa-d99b73ef2adan%40googlegroups.com.

[Brian Candler]

> that is an atomicity issue

Exactly.  If you build something that does atomic reads and writes only, you are generally ignoring the larger picture of what sequence of operations needs to be atomic.

> Also the `value` entry in the struct is in small cases, prohibiting getting accessed when imported, so the side-attack to directly call `v.value` won't pass the compile phase, which is much safer than the conventional `atomic` package.

Sorry, but I don't understand what you are saying at all.

The operations in sync.Atomic work on individual values. They don't need a separate mutex, so you don't need to carry the value in a struct in the first place. What kind of "safety" are you referring to? If you export Load() and Save() methods then the value is available to read and write by anyone.

[Zhaoxun Yan]

The operations in sync.Atomic work on individual values. They don't need a separate mutex, so you don't need to carry the value in a struct in the first place... What kind of "safety" are you referring to?

Here is the risk of using Atomic:

===========================

WARNING: DATA RACE
Write at 0x0000016a4808 by goroutine 91:
  sync/atomic.StoreInt64()
      /usr/go15/src/runtime/race_amd64.s:234 +0xb
  quotelive/util.Zero()
      /home/zxun/src/quotelive/util/util.go:89 +0xbe
【atomic.StoreInt64(a, 0)】
  main.startCTP()
      /home/zxun/src/quotelive/ctpschedule.go:279 +0xa5
【util.Zero(&util.CountQ)】

  github.com/robfig/cron.FuncJob.Run()
      /home/zxun/src/github.com/robfig/cron/cron.go:92 +0x34
  github.com/robfig/cron.(*Cron).runWithRecovery()
      /home/zxun/src/github.com/robfig/cron/cron.go:165 +0x72

Previous read at 0x0000016a4808 by goroutine 31:
  main.ctpDaemon()
      /home/zxun/src/quotelive/ctp.go:455 +0x257
【oldcount = util.CountQ】

Goroutine 91 (running) created at:
  github.com/robfig/cron.(*Cron).run()
      /home/zxun/src/github.com/robfig/cron/cron.go:199 +0xa90

Goroutine 31 (running) created at:
  main.initQuote()
      /home/zxun/src/quotelive/ctpschedule.go:428 +0xe4
  main.main()
      /home/zxun/src/quotelive/main.go:56 +0x251

=======================

util.CountQ is a simple int64, so a programmer can access it by conventional means '=' or '==' or '>', etc.

But that access itself is already a reading behavior which caused race.

Even every writing of this variable applied atomic.StoreInt64 won't prohibit this kind of side-attack (mistake) .

In contrast, if I import the struct type from an independent package like 'lock' or 'global', I cannot read or write the `value` entry of that type of struct without the existing methods in that package such as `Load`, `Save` or `Tick`, prohibiting side-attack (mistake) . Because `value` with initial not-capitalized is a private entry of the struct and is forbidden outside that package, while capitalized methods are public and available for call outside it. This is one advantage of using my proposal. Additional pro is method namings are shorter.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/52f27763-3c40-457a-8c6b-850e2d74108en%40googlegroups.com.

[Zhaoxun Yan]

// You can edit this code!
// Click here and start typing.

package main

import (
"fmt"
"sync/atomic"
)

var a = &atomic.Int32{}  【【./prog.go:10:17: undefined: atomic.Int32

func main() {
fmt.Println(a.Load())
}

[Axel Wagner]

Yes, as I said, you are apparently not using tip. You can set that up in the playground, there is a drop-down beside the `Run` button, you can set that to `go dev branch`. In a previous message, you got a different error message, which implies that you did, at least at some point, use tip.

If you want to use these types with Go 1.18, you can copy them from the atomic package. e.g. copy these lines:

https://cs.opensource.google/go/go/+/master:src/sync/atomic/type.go;l=58-79

Of course you have to import `sync/atomic` and add `atomic.` to all the relative function calls (e.g. replace `LoadInt32` with `atomic.LoadInt32`).

[Axel Wagner]

Also, FWIW, there is no need to write `&atomic.Int32{}`. You can just do `var a atomic.Int32` and call `a.Load()`, the compiler de-sugars that automatically into the appropriate call.

[Zhaoxun Yan]

Yes it worked in the 'Go Dev Branch'.

Hmm, it seems my proposal gets accepted in the latest version of atomic.

Now atomic.Int32 is a struct, too.