Skip to content

Refactor #991 data race fixes#995

Merged
cgalibern merged 5 commits intoopensvc:mainfrom
cgalibern:dev
Mar 31, 2026
Merged

Refactor #991 data race fixes#995
cgalibern merged 5 commits intoopensvc:mainfrom
cgalibern:dev

Conversation

@cgalibern
Copy link
Copy Markdown
Contributor

This pull request partially revisits #75a427cf70665aed275931a6d7392eee4e7c6717 & #b5ee8a80201d029750652aa753973c3d5151e875

This commit partially revisits #75a427cf70665aed275931a6d7392eee4e7c6717
Arguments passed to func (data *ClusterData) onXX functions must be deeply copied beforehand.

- Data Set functions receives deep-copied arguments:
   instance.MonitorData.Set(t.path, t.localhost, t.state.DeepCopy())
- The publication message also uses a deep copy:
   t.publisher.Pub(&msgbus.InstanceMonitorUpdated{Path: t.path, Value: *t.state.DeepCopy()}...
This commit deeply copies status when selector or namespace filtering is used.

## DATA RACE example:
    ==================
    WARNING: DATA RACE
    Read at 0x00c0043f4ed0 by goroutine 45997:
      github.com/opensvc/om3/v3/core/clusterdump.(*Data).ObjectPaths()
          ./om3/core/clusterdump/cluster.go:68 +0x6e
      github.com/opensvc/om3/v3/core/clusterdump.(*Data).WithSelector()
          ./om3/core/clusterdump/cluster.go:85 +0x64
      github.com/opensvc/om3/v3/daemon/daemonapi.(*DaemonAPI).GetClusterStatus()
          ./om3/daemon/daemonapi/get_daemon_status.go:50 +0x3fe
      github.com/opensvc/om3/v3/daemon/api.(*ServerInterfaceWrapper).GetClusterStatus()
          ./om3/daemon/api/codegen_server_gen.go:855 +0x474
      github.com/opensvc/om3/v3/daemon/api.(*ServerInterfaceWrapper).GetClusterStatus-fm()
          <autogenerated>:1 +0x47
      github.com/labstack/echo/v4.(*Echo).add.func1()
          ./go/pkg/mod/github.com/labstack/echo/v4@v4.13.4/echo.go:581 +0x76
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.LogRequestMiddleWare.func3.1()
          ./om3/daemon/daemonapi/lib_middleware.go:264 +0x564
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.LogUserMiddleware.func2.1()
          ./om3/daemon/daemonapi/lib_middleware.go:231 +0x1b9
      github.com/opensvc/om3/v3/daemon/daemonapi.AuthMiddleware.func3.1()
          ./om3/daemon/daemonapi/lib_middleware.go:212 +0xf1e
      github.com/opensvc/om3/v3/daemon/daemonapi.LogMiddleware.func1.1()
          ./om3/daemon/daemonapi/lib_middleware.go:141 +0x544
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.func1.1()
          ./om3/daemon/listener/routehttp/main.go:68 +0x224
      github.com/labstack/echo-contrib/echoprometheus.MiddlewareConfig.ToMiddleware.func3.1()
          ./go/pkg/mod/github.com/labstack/echo-contrib@v0.17.4/echoprometheus/prometheus.go:252 +0x461
      github.com/labstack/echo/v4.(*Echo).ServeHTTP()
          ./go/pkg/mod/github.com/labstack/echo/v4@v4.13.4/echo.go:668 +0x7b4
      github.com/opensvc/om3/v3/daemon/listener/routehttp.(*T).ServeHTTP()
          ./om3/daemon/listener/routehttp/main.go:84 +0x53
      net/http.Handler.ServeHTTP-fm()
          <autogenerated>:1 +0x67
      golang.org/x/net/http2.(*serverConn).runHandler()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2424 +0x1f3
      golang.org/x/net/http2.(*serverConn).scheduleHandler.gowrap1()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2359 +0x5d

    Previous write at 0x00c0043f4ed0 by goroutine 45967:
      runtime.mapassign()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/internal/runtime/maps/runtime_swiss.go:191 +0x0
      github.com/opensvc/om3/v3/core/clusterdump.(*Data).WithSelector()
          ./om3/core/clusterdump/cluster.go:100 +0x404
      github.com/opensvc/om3/v3/daemon/daemonapi.(*DaemonAPI).GetClusterStatus()
          ./om3/daemon/daemonapi/get_daemon_status.go:50 +0x3fe
      github.com/opensvc/om3/v3/daemon/api.(*ServerInterfaceWrapper).GetClusterStatus()
          ./om3/daemon/api/codegen_server_gen.go:855 +0x474
      github.com/opensvc/om3/v3/daemon/api.(*ServerInterfaceWrapper).GetClusterStatus-fm()
          <autogenerated>:1 +0x47
      github.com/labstack/echo/v4.(*Echo).add.func1()
          ./go/pkg/mod/github.com/labstack/echo/v4@v4.13.4/echo.go:581 +0x76
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.LogRequestMiddleWare.func3.1()
          ./om3/daemon/daemonapi/lib_middleware.go:264 +0x564
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.LogUserMiddleware.func2.1()
          ./om3/daemon/daemonapi/lib_middleware.go:231 +0x1b9
      github.com/opensvc/om3/v3/daemon/daemonapi.AuthMiddleware.func3.1()
          ./om3/daemon/daemonapi/lib_middleware.go:212 +0xf1e
      github.com/opensvc/om3/v3/daemon/daemonapi.LogMiddleware.func1.1()
          ./om3/daemon/daemonapi/lib_middleware.go:141 +0x544
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.func1.1()
          ./om3/daemon/listener/routehttp/main.go:68 +0x224
      github.com/labstack/echo-contrib/echoprometheus.MiddlewareConfig.ToMiddleware.func3.1()
          ./go/pkg/mod/github.com/labstack/echo-contrib@v0.17.4/echoprometheus/prometheus.go:252 +0x461
      github.com/labstack/echo/v4.(*Echo).ServeHTTP()
          ./go/pkg/mod/github.com/labstack/echo/v4@v4.13.4/echo.go:668 +0x7b4
      github.com/opensvc/om3/v3/daemon/listener/routehttp.(*T).ServeHTTP()
          ./om3/daemon/listener/routehttp/main.go:84 +0x53
      net/http.Handler.ServeHTTP-fm()
          <autogenerated>:1 +0x67
      golang.org/x/net/http2.(*serverConn).runHandler()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2424 +0x1f3
      golang.org/x/net/http2.(*serverConn).scheduleHandler.gowrap1()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2359 +0x5d

    Goroutine 45997 (running) created at:
      golang.org/x/net/http2.(*serverConn).scheduleHandler()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2359 +0x238
      golang.org/x/net/http2.(*serverConn).processHeaders()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2116 +0xc84
      golang.org/x/net/http2.(*serverConn).processFrame()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1609 +0x2f5
      golang.org/x/net/http2.(*serverConn).processFrameFromReader()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1547 +0x2e4
      golang.org/x/net/http2.(*serverConn).serve()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1003 +0x18a4
      golang.org/x/net/http2.(*Server).serveConn()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:565 +0x1aad
      golang.org/x/net/http2.(*Server).ServeConn()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:424 +0x924
      golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/h2c/h2c.go:97 +0x815
      golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/h2c/h2c.go:89 +0x5a8
      golang.org/x/net/http2/h2c.(*h2cHandler).ServeHTTP()
          <autogenerated>:1 +0x74
      net/http.serverHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:3301 +0x2a1
      net/http.(*conn).serve()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:2102 +0x1304
      net/http.(*Server).Serve.gowrap3()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:3454 +0x4f

    Goroutine 45967 (running) created at:
      golang.org/x/net/http2.(*serverConn).scheduleHandler()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2359 +0x238
      golang.org/x/net/http2.(*serverConn).processHeaders()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2116 +0xc84
      golang.org/x/net/http2.(*serverConn).processFrame()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1609 +0x2f5
      golang.org/x/net/http2.(*serverConn).processFrameFromReader()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1547 +0x2e4
      golang.org/x/net/http2.(*serverConn).serve()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1003 +0x18a4
      golang.org/x/net/http2.(*Server).serveConn()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:565 +0x1aad
      golang.org/x/net/http2.(*Server).ServeConn()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:424 +0x924
      golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/h2c/h2c.go:97 +0x815
      golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/h2c/h2c.go:89 +0x5a8
      golang.org/x/net/http2/h2c.(*h2cHandler).ServeHTTP()
          <autogenerated>:1 +0x74
      net/http.serverHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:3301 +0x2a1
      net/http.(*conn).serve()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:2102 +0x1304
      net/http.(*Server).Serve.gowrap3()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:3454 +0x4f
    ==================
…tion

Introduced `sync.RWMutex` to `ReadCloser` for protecting access to the `closed`
field and ensuring thread-safe execution during close operations.

## Fixed DATA RACE:
     Write at 0x00c000206b81 by goroutine 282:
       github.com/opensvc/om3/v3/core/event/sseevent.(*ReadCloser).Close()
           ./om3/core/event/sseevent/main.go:154 +0x9e
       github.com/opensvc/om3/v3/core/objectaction.T.waitExpectation.func12.2()
           ./om3/core/objectaction/object.go:1143 +0x62

     Previous read at 0x00c000206b81 by goroutine 280:
       github.com/opensvc/om3/v3/core/event/sseevent.(*ReadCloser).Read()
           ./om3/core/event/sseevent/main.go:109 +0x59
       github.com/opensvc/om3/v3/core/objectaction.T.waitExpectation.func12()
           ./om3/core/objectaction/object.go:1152 +0x3e3

     Goroutine 282 (running) created at:
       github.com/opensvc/om3/v3/core/objectaction.T.waitExpectation.func12()
           ./om3/core/objectaction/object.go:1139 +0x2d8

     Goroutine 280 (running) created at:
       github.com/opensvc/om3/v3/core/objectaction.T.waitExpectation()
           ./om3/core/objectaction/object.go:1128 +0x2344
       github.com/opensvc/om3/v3/core/objectaction.T.DoAsync()
           ./om3/core/objectaction/object.go:458 +0x7d2
       github.com/opensvc/om3/v3/core/objectaction.(*T).DoAsync()
           <autogenerated>:1 +0x9b
       github.com/opensvc/om3/v3/core/actionrouter.Do()
           ./om3/core/actionrouter/action.go:150 +0x148
       github.com/opensvc/om3/v3/core/objectaction.T.Do()
           ./om3/core/objectaction/object.go:964 +0xe3e
       github.com/opensvc/om3/v3/core/omcmd.(*CmdObjectStart).Run()
           ./om3/core/omcmd/object_start.go:118 +0xded
       github.com/opensvc/om3/v3/core/om.newCmdObjectStart.func1()
           ./om3/core/om/factory.go:2990 +0x46
       github.com/spf13/cobra.(*Command).execute()
           ./go/pkg/mod/github.com/spf13/cobra@v1.9.1/command.go:1015 +0x10bb
       github.com/spf13/cobra.(*Command).ExecuteC()
           ./go/pkg/mod/github.com/spf13/cobra@v1.9.1/command.go:1148 +0x647
       github.com/spf13/cobra.(*Command).Execute()
           ./go/pkg/mod/github.com/spf13/cobra@v1.9.1/command.go:1071 +0x9c
       github.com/opensvc/om3/v3/core/om.ExecuteArgs()
           ./om3/core/om/root.go:218 +0x12
       github.com/opensvc/om3/v3/core/om.Execute()
           ./om3/core/om/root.go:158 +0x5b
       main.main()
           ./om3/cmd/om/main.go:29 +0x64
Introduced a deep copy of `GlobalExpectOptions` to prevent concurrent writes/reads while updating destination values.

## data race example:
    Write at 0x00c00341cb90 by goroutine 296824:
      github.com/opensvc/om3/v3/daemon/imon.(*Manager).onSetInstanceMonitor.func3()
          ./om3/daemon/imon/main_cmd.go:527 +0x16f1
      github.com/opensvc/om3/v3/daemon/imon.(*Manager).onSetInstanceMonitor()
          ./om3/daemon/imon/main_cmd.go:618 +0x7a7
      github.com/opensvc/om3/v3/daemon/imon.(*Manager).worker()
          ./om3/daemon/imon/main.go:452 +0x1544
      github.com/opensvc/om3/v3/daemon/imon.start.func1()
          ./om3/daemon/imon/main.go:284 +0x54

    Previous read at 0x00c00341cb90 by goroutine 289334:
      reflect.Value.IsNil()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/reflect/value.go:1556 +0x9e
      encoding/json.interfaceEncoder()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/encoding/json/encode.go:676 +0xab
      encoding/json.structEncoder.encode()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/encoding/json/encode.go:727 +0x3c7
      encoding/json.structEncoder.encode-fm()
          <autogenerated>:1 +0xe4
      encoding/json.structEncoder.encode()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/encoding/json/encode.go:727 +0x3c7
      encoding/json.structEncoder.encode-fm()
          <autogenerated>:1 +0xe4
      encoding/json.ptrEncoder.encode()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/encoding/json/encode.go:899 +0x3d1
      encoding/json.ptrEncoder.encode-fm()
          <autogenerated>:1 +0x84
      encoding/json.(*encodeState).reflectValue()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/encoding/json/encode.go:333 +0x83
      encoding/json.(*encodeState).marshal()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/encoding/json/encode.go:309 +0xea
      encoding/json.Marshal()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/encoding/json/encode.go:175 +0x12b
      github.com/opensvc/om3/v3/core/event.ToEvent()
          ./om3/core/event/main.go:84 +0x2b3
      github.com/opensvc/om3/v3/daemon/daemonapi.(*DaemonAPI).getLocalDaemonEvents.func6()
          ./om3/daemon/daemonapi/get_daemon_events.go:359 +0x96
      github.com/opensvc/om3/v3/daemon/daemonapi.(*DaemonAPI).getLocalDaemonEvents()
          ./om3/daemon/daemonapi/get_daemon_events.go:483 +0x375b
      github.com/opensvc/om3/v3/daemon/daemonapi.(*DaemonAPI).GetDaemonEvents()
          ./om3/daemon/daemonapi/get_daemon_events.go:61 +0x189
      github.com/opensvc/om3/v3/daemon/api.(*ServerInterfaceWrapper).GetDaemonEvents()
          ./om3/daemon/api/codegen_server_gen.go:1712 +0xb56
      github.com/opensvc/om3/v3/daemon/api.(*ServerInterfaceWrapper).GetDaemonEvents-fm()
          <autogenerated>:1 +0x47
      github.com/labstack/echo/v4.(*Echo).add.func1()
          ./go/pkg/mod/github.com/labstack/echo/v4@v4.13.4/echo.go:581 +0x76
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.LogRequestMiddleWare.func3.1()
          ./om3/daemon/daemonapi/lib_middleware.go:264 +0x564
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.LogUserMiddleware.func2.1()
          ./om3/daemon/daemonapi/lib_middleware.go:231 +0x1b9
      github.com/opensvc/om3/v3/daemon/daemonapi.AuthMiddleware.func3.1()
          ./om3/daemon/daemonapi/lib_middleware.go:212 +0xf1e
      github.com/opensvc/om3/v3/daemon/daemonapi.LogMiddleware.func1.1()
          ./om3/daemon/daemonapi/lib_middleware.go:141 +0x544
      github.com/opensvc/om3/v3/daemon/listener/routehttp.New.func1.1()
          ./om3/daemon/listener/routehttp/main.go:68 +0x224
      github.com/labstack/echo-contrib/echoprometheus.MiddlewareConfig.ToMiddleware.func3.1()
          ./go/pkg/mod/github.com/labstack/echo-contrib@v0.17.4/echoprometheus/prometheus.go:252 +0x461
      github.com/labstack/echo/v4.(*Echo).ServeHTTP()
          ./go/pkg/mod/github.com/labstack/echo/v4@v4.13.4/echo.go:668 +0x7b4
      github.com/opensvc/om3/v3/daemon/listener/routehttp.(*T).ServeHTTP()
          ./om3/daemon/listener/routehttp/main.go:84 +0x53
      net/http.Handler.ServeHTTP-fm()
          <autogenerated>:1 +0x67
      golang.org/x/net/http2.(*serverConn).runHandler()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2424 +0x1f3
      golang.org/x/net/http2.(*serverConn).scheduleHandler.gowrap1()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2359 +0x5d

    Goroutine 296824 (running) created at:
      github.com/opensvc/om3/v3/daemon/imon.start()
          ./om3/daemon/imon/main.go:283 +0x1318
      github.com/opensvc/om3/v3/daemon/imon.Factory.Start()
          ./om3/daemon/imon/main.go:204 +0x105
      github.com/opensvc/om3/v3/daemon/imon.(*Factory).Start()
          <autogenerated>:1 +0x29
      github.com/opensvc/om3/v3/daemon/omon.(*Manager).startInstanceMonitor()
          ./om3/daemon/omon/main.go:550 +0x238
      github.com/opensvc/om3/v3/daemon/omon.(*Manager).worker()
          ./om3/daemon/omon/main.go:205 +0xe35
      github.com/opensvc/om3/v3/daemon/omon.Start.func1()
          ./om3/daemon/omon/main.go:151 +0x76

    Goroutine 289334 (running) created at:
      golang.org/x/net/http2.(*serverConn).scheduleHandler()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2359 +0x238
      golang.org/x/net/http2.(*serverConn).processHeaders()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:2116 +0xc84
      golang.org/x/net/http2.(*serverConn).processFrame()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1609 +0x2f5
      golang.org/x/net/http2.(*serverConn).processFrameFromReader()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1547 +0x2e4
      golang.org/x/net/http2.(*serverConn).serve()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:1003 +0x18a4
      golang.org/x/net/http2.(*Server).serveConn()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:565 +0x1aad
      golang.org/x/net/http2.(*Server).ServeConn()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/server.go:424 +0x924
      golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/h2c/h2c.go:97 +0x815
      golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/x/net@v0.47.0/http2/h2c/h2c.go:89 +0x5a8
      golang.org/x/net/http2/h2c.(*h2cHandler).ServeHTTP()
          <autogenerated>:1 +0x74
      net/http.serverHandler.ServeHTTP()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:3301 +0x2a1
      net/http.(*conn).serve()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:2102 +0x1304
      net/http.(*Server).Serve.gowrap3()
          ./go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.linux-amd64/src/net/http/server.go:3454 +0x4f
    ==================
@cgalibern cgalibern merged commit be8b12a into opensvc:main Mar 31, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant