When set time.Location, will be detected Data Race

[zhangzh...@gmail.com]

My go version is 1.11.

When I set time.Location, then …

==================
WARNING: DATA RACE
Write at 0x000002e275d0 by main goroutine:
  vcs.taiyouxi.net/platform/planx/timeutil.SetTimeLocal()
      /Users/zhangzhen/serverthreekingdom/src/vcs.taiyouxi.net/platform/planx/timeutil/time_util.go:51 +0xb4
  vcs.taiyouxi.net/platform/planx/timeutil.init.0()
      /Users/zhangzhen/serverthreekingdom/src/vcs.taiyouxi.net/platform/planx/timeutil/time_util.go:42 +0x43
  vcs.taiyouxi.net/platform/planx/timeutil.init()
      <autogenerated>:1 +0xd0
  vcs.taiyouxi.net/jws2/common/time.init()
      <autogenerated>:1 +0xa6
  vcs.taiyouxi.net/jws2/gamex/account/account.init()
      <autogenerated>:1 +0xa6
  vcs.taiyouxi.net/jws2/gamex/logics.init()
      <autogenerated>:1 +0xa6
  vcs.taiyouxi.net/jws2/gamex/cmds/gamemode.init()
      <autogenerated>:1 +0xa6
  main.init()
      <autogenerated>:1 +0xa6

Previous read at 0x000002e275d0 by goroutine 8:
  time.Now()
      /Users/zhangzhen/.gvm/gos/go1.11/src/time/time.go:1060 +0xcf
  time.sendTime()
      /Users/zhangzhen/.gvm/gos/go1.11/src/time/sleep.go:141 +0x44

Goroutine 8 (running) created at:
  runtime.(*timersBucket).addtimerLocked()
      /Users/zhangzhen/.gvm/gos/go1.11/src/runtime/time.go:170 +0x113
  vcs.taiyouxi.net/vendor/github.com/siddontang/go/timingwheel.NewTimingWheel()
      /Users/zhangzhen/serverthreekingdom/src/vcs.taiyouxi.net/vendor/github.com/siddontang/go/timingwheel/timingwheel.go:39 +0x2a0
  vcs.taiyouxi.net/platform/planx/util.init()
      /Users/zhangzhen/serverthreekingdom/src/vcs.taiyouxi.net/platform/planx/util/timer_helper.go:10 +0xf3
  vcs.taiyouxi.net/platform/planx/metrics.init()
      <autogenerated>:1 +0xbf
  vcs.taiyouxi.net/jws2/gamex/cmds/gamemode.init()
      <autogenerated>:1 +0x9c
  main.init()
      <autogenerated>:1 +0xa6
==================

The feature of golang is goroutine, and it is a normal situation that get time in different goroutine.
But only can set time.Location in one goroutine. So the Data Race is inevitable.

And there is the replay from agnivade 3

“It is not a bug. Please synchronize access to time.Location using synchronization primitives in the sync andsync/atomic packages.”

But there are lots of place that time.Location used by time package of golang. For example: time.Now(), time.locabs()
I can’t change them.

So, What should I do ? Please help me.

[Ian Lance Taylor]

You said time.Location. Do you mean time.Local? It's true that you
should never change time.Local. Why do you want to? If you want to
change the location of a specific time.Time value, call the In method.

Ian

[Marvin Renich]

* Ian Lance Taylor <ia...@golang.org> [181130 01:01]:

For the OP's clarification, I believe he is trying to take the result of
t.Location(), which is a *time.Location, and happens to be the time.loc
private member of the Time struct (unless that is nil, in which case the
result is time.UTC), and assign to it.

However, your answer still stands.

...Marvin

[Agniva De Sarker]

Can you show us the code ? It would help 

Are you trying to set a variable of type time.Location ? Or are you trying to set time.Local to something else ?

Like Ian said, if you want to change the location of a specific time value, use the In method.

> But there are lots of place that time.Location used by time package of golang. For example: time.Now(), time.locabs()

> I can’t change them.

I don't understand. Use time.Now() uses the time.Local variable. But all you need to do is wrap the setting of time.Local value with a mutex. Why is it not possible to do that ?

[Matt Harden]

Just wrapping a write with a mutex is not enough; you have to wrap all the reads as well. In this case that's not possible because there are reads of time.Local all over the place. Anyway, as has been said, time.Local should never be written to in user code.

--
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.
For more options, visit https://groups.google.com/d/optout.

[scud...@gmail.com]

A bit late to the party here but I would like to explore your rational for stating that time.Local should not be modified by user code. The code seems to suggest this is expected:

https://golang.org/src/time/zoneinfo.go

```

// localLoc is separate so that initLocal can initialize
    76  // it even if a client has changed Local.

```

I want to change Local because I want all JSON encoding to be normalized to UTC and not local time - there does not seem to be a way to do this (please let me know if I am missing something). It is possible to set the TZ env var but not from within the program itself - it has to be done via a hacky shell script before I launch the program. This is because it is impossible to guarantee that my code will run before any library might access any time functions causing initLocal to fire. After this changing the TZ env var makes no difference.

I know I can set the Location in every time.Time object I create but this is impossible for time.Time objects coming from other libraries. I essentially need to write my own visitor to try to identify such foreign time.Time objects and fix their Location before they can be json serialized.

I tried to update time.Local using atomic.SetPointer so there really is no race but the race detector still complains - if there a way to turn it off?

Thanks

Mike

On Thursday, December 6, 2018 at 3:38:36 PM UTC+10, Matt Harden wrote:

Just wrapping a write with a mutex is not enough; you have to wrap all the reads as well. In this case that's not possible because there are reads of time.Local all over the place. Anyway, as has been said, time.Local should never be written to in user code.

To unsubscribe from this group and stop receiving emails from it, send an email to golan...@googlegroups.com.

[Ian Lance Taylor]

On Thu, Jul 23, 2020 at 9:47 AM <scud...@gmail.com> wrote:
>
> A bit late to the party here but I would like to explore your rational for stating that time.Local should not be modified by user code. The code seems to suggest this is expected:
>
> https://golang.org/src/time/zoneinfo.go
> ```
>
> // localLoc is separate so that initLocal can initialize
> 76 // it even if a client has changed Local.
>
> ```
>
> I want to change Local because I want all JSON encoding to be normalized to UTC and not local time - there does not seem to be a way to do this (please let me know if I am missing something). It is possible to set the TZ env var but not from within the program itself - it has to be done via a hacky shell script before I launch the program. This is because it is impossible to guarantee that my code will run before any library might access any time functions causing initLocal to fire. After this changing the TZ env var makes no difference.
>
> I know I can set the Location in every time.Time object I create but this is impossible for time.Time objects coming from other libraries. I essentially need to write my own visitor to try to identify such foreign time.Time objects and fix their Location before they can be json serialized.
>
> I tried to update time.Local using atomic.SetPointer so there really is no race but the race detector still complains - if there a way to turn it off?

There is no way to change time.Local. It is accessed without any
protection, because the time package assumes that it will not change.

If your program doesn't fetch any timezones at initialization time,
you could use

func init() {
os.Setenv("TZ", "")
}

Ian

[Michael Cohen]

Thanks Ian, this does not work as I mentioned because there is no way to guarantee that my init function will run before any of my libraries creating a time object (which many do as they start various background tasks). 

Doing what you say is fragile because even if it appears to work, one day I might link a library and it will mysteriously break randomly (this happened to us previously).

Is this a bug in the time package? Or simply a bug or design shortfall in the json package? One way I thought to fix this issue is to simply fork the json encoder and handle time.Time structs specifically, but this obviously does not scale to eg yaml encoders etc.

Thanks

Mike

[Jason E. Aten]

> I want to change Local because I want all JSON encoding to be normalized to UTC and not local time - there does not seem to be a way to do this (please let me know if I am missing something). 

Yes, you are missing something. This is very easy. Do not change Local. Just use the In() method on any time.Time. Pass it the *time.Location constant for the place you want. For UTC, simply use time.UTC

currentTimeInUTC := time.Now().In(time.UTC) 

[Ian Lance Taylor]

On Thu, Jul 23, 2020 at 5:13 PM Michael Cohen <scud...@gmail.com> wrote:
>
> Thanks Ian, this does not work as I mentioned because there is no way to guarantee that my init function will run before any of my libraries creating a time object (which many do as they start various background tasks).
>
> Doing what you say is fragile because even if it appears to work, one day I might link a library and it will mysteriously break randomly (this happened to us previously).
>
> Is this a bug in the time package? Or simply a bug or design shortfall in the json package? One way I thought to fix this issue is to simply fork the json encoder and handle time.Time structs specifically, but this obviously does not scale to eg yaml encoders etc.

I don't think it's a shortfall in the encoding/json package as such,
as that package knows nothing about times. It's the time package that
implements an UnmarshalJSON. Looking at that method, I'm a little
surprised at what you describe, since it just uses the timezone in the
JSON string. It doesn't use the local timezone as far as I can see.
Can you show a small program that demonstrates the problem?

Ian

[Mike Cohen]

To be clear, the time.MarshalJSON() function produces valid RFC 3339 timestamps which allow arbitrary timezones. This means that local time offset is used in the string representation. Although all representations are technically equivalent their string form is not so we need to do special parsing to compare them (and users get really confused as well).

Here is an example - 

https://play.golang.org/p/hELVEI1tGJd

when I run it I get {"A":6,"B":"2009-11-11T10:00:00+11:00"}  for Australia time, but

 {"A":6,"B":"2009-11-10T18:00:00-05:00"} for America/New_York.

I am aware I can use the .In() method to cast the time.Time object into different timezone but this is the whole issue - I need to search through all the struct fields, expand slices, recurse through maps etc just to find any time.Time objects and change them **before** I call json.Marshal().

I usually get the times from a struct in another library so I have no control over how these times are made for example consider this

https://play.golang.org/p/HmGREBF2Hbd

where I call an out of tree library to build a struct then just want to dump it - I have no control over the encoding.

I think this is a actually a limitation in the json strategy of encoding based only on types and not on context. The TZ issue is just a manifestation of that - even if there was a safe way to set TZ in a running program, in a multithreaded program all json encoders would be forced to use the same local TZ because encoding goes with type and not with context.

Thanks.

Mike

-- 

Mike Cohen Digital Paleontologist Velocidex Enterprises https://www.velocidex.com +61.470238491

[Robert Engels]

Yea, it is strange the the Json encoder api doesn’t allow the caller to provide a mashaller for any type. It would be trivial to change the code to do so. 

That being said I’m sure you can find custom json libs on the web that will do this. Or fork it yourself - it is not a lot of code. 

On Jul 24, 2020, at 11:15 AM, Mike Cohen <mi...@velocidex.com> wrote:



-- 

<velo_black_small_full_res.png>

Mike Cohen Digital Paleontologist Velocidex Enterprises https://www.velocidex.com +61.470238491

--
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/55b1098d4ff2518143b249b4587b36b527797896.camel%40velocidex.com.

[Steven Hartland]

Always serialise t.UTC()?

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

[Mike Cohen]

Thanks Robert, this is what I ended up doing. I wouldnt say it was trivial though

https://github.com/Velocidex/json

Thanks and sorry for hijacking this thread. Maybe a suggestion - if users are not expected to change time.Local maybe it should be made private or at least wording in the module should not say that clients are allowed to change it. Having a public variable invites users to expect that changing it is acceptable and only after getting hit by the race detector and spending a huge amount of time figuring out the details it becomes apparent that it should not be changed

Thanks

Mike

--