Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition within pID get #1131

Open
GetSource1234 opened this issue Nov 11, 2024 · 1 comment
Open

Race condition within pID get #1131

GetSource1234 opened this issue Nov 11, 2024 · 1 comment

Comments

@GetSource1234
Copy link

GetSource1234 commented Nov 11, 2024

There is a race condition when getting the process ID right away after the cluster starts. It seems .onClusterTopology() conflicts with cluster.Get().

To Reproduce

  • Create a localhost cluster (only one node) with default Consul
  • Get process ID (i.e. c.Get("abc", "hello"))
  • run code with race detection

Test code:


import (
	"fmt"

	"github.com/asynkron/protoactor-go/actor"
	"github.com/asynkron/protoactor-go/cluster"
	"github.com/asynkron/protoactor-go/cluster/clusterproviders/consul"
	"github.com/asynkron/protoactor-go/cluster/identitylookup/disthash"
	"github.com/asynkron/protoactor-go/remote"
)

type HelloRequest struct {
	Name string
}

func main() {
	c := startNode()

	fmt.Print("\nBoot other nodes and press Enter\n")
	pid := c.Get("abc", "hello")
	fmt.Printf("Got pid %v", pid)
	res, _ := c.Request("abc", "hello", HelloRequest{Name: "Roger"})
	fmt.Printf("Got response %v", res)

	fmt.Println()
	c.Shutdown(true)
}

func startNode() *cluster.Cluster {
	system := actor.NewActorSystem()

	provider, _ := consul.New()
	lookup := disthash.New()
	config := remote.Configure("localhost", 0)
	clusterConfig := cluster.Configure("my-cluster", provider, lookup, config)
	c := cluster.New(system, clusterConfig)
	c.StartMember()

	return c
}

Cmd to run:
go run -race main.go

Output:

WARNING: DATA RACE
Read at 0x00c000710098 by main goroutine:
  github.com/asynkron/protoactor-go/cluster/identitylookup/disthash.(*Manager).Get()
      /..../go/pkg/mod/github.com/asynkron/[email protected]/cluster/identitylookup/disthash/manager.go:75 +0x44
  github.com/asynkron/protoactor-go/cluster/identitylookup/disthash.(*IdentityLookup).Get()
  
  Previous write at 0x00c000710098 by goroutine 39:
  github.com/asynkron/protoactor-go/cluster/identitylookup/disthash.(*Manager).onClusterTopology()
      /..../go/pkg/mod/github.com/asynkron/[email protected]/cluster/identitylookup/disthash/manager.go:69 +0x464
  github.com/asynkron/protoactor-go/cluster/identitylookup/disthash.(*Manager).Start.func2()

Env:
MacOS Monterey v12.7.6
MacBook Pro mid 2015
Go v1.23.0

Expected behavior
Process ID creation or retrieval should not lead to race conditions to avoid data inconsistency.

@GetSource1234
Copy link
Author

Do you have any thoughts or updates? I did not dive deep in the code, but I suppose we must sync rdv, before retrieval if an async getter is used, i.e.
pm.rdv = clustering.NewRendezvous() -> ownerAddress := pm.rdv.GetByClusterIdentity(identity)

BTW: I noticed that the last commit was quite a time ago, and I wanted to check if the project is still actively maintained.

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

No branches or pull requests

1 participant