rpc: keep published port lookups alive when datapool stalls

[tuhalf]

This pull request refactors how published ports are stored and accessed in the DataPool to improve thread safety and reliability, especially when the actor is blocked. The main change is moving the published ports state to an atomic.Value for lock-free reads, ensuring that GetPublishedPort does not block or return incorrect results if the actor is wedged. Additionally, new tests are added to verify this behavior and to ensure the published ports map is safely copied.

Thread safety and published ports state:

• Introduced a new atomic.Value field publishedState in DataPool to hold the published ports map, enabling lock-free and reliable access from GetPublishedPort. (rpc/datapool.go) [1] [2]
• Updated NewPool to initialize publishedState with an empty map. (rpc/datapool.go)
• Modified SetPublishedPorts to copy the input map before storing it in publishedState, ensuring external changes do not affect the internal state. (rpc/datapool.go)
• Changed GetPublishedPort to read from publishedState instead of routing through the actor, avoiding actor deadlocks and false negatives. (rpc/datapool.go)

Testing and reliability:

• Added new tests in rpc/datapool_test.go to verify that GetPublishedPort returns correct results even if the actor is blocked, and to ensure that SetPublishedPorts creates a snapshot copy of the input map.

[gemini-code-assist]

Code Review

This pull request updates the DataPool to use atomic.Value for storing published ports, ensuring that GetPublishedPort remains non-blocking. It also implements map snapshotting in SetPublishedPorts to prevent external mutations and adds regression tests. Review feedback suggests using a safer two-value type assertion when loading from atomic.Value to avoid potential panics and recommends removing the now-redundant publishedPorts field.

[gemini-code-assist]

The single-value type assertion p.publishedState.Load().(map[int]*config.Port) will cause a panic if the atomic.Value ever holds a different type. It's safer to use the two-value form to check if the type assertion is successful.

Suggested change

	state, _ := p.publishedState.Load().(map[int]*config.Port)
	if state == nil {
	return nil
	}
	port = state[portnum]
	state, ok := p.publishedState.Load().(map[int]*config.Port)
	if !ok {
	return nil
	}
	port = state[portnum]

[gemini-code-assist]

With GetPublishedPort now using publishedState, the publishedPorts field appears to be redundant. It's initialized and written to, but never read. To simplify the code and remove unnecessary state, you could remove the publishedPorts field from the DataPool struct, its initialization in NewPool, and this p.srv.Cast call.

[dominicletz]

I don't see the improvement here for the issue Hans explained. Let's focus instead on two improvements here:

1. Any call to SetPublishedPorts should show a debug log
2. Test running one or two local diode_node servers, connect the CLI to them and then kill one of them to reproduce what Hans has been seeing.

#271

[dominicletz]

I don't see the improvement here for the issue Hans explained. Let's focus instead on two improvements here:

Any call to SetPublishedPorts should show a debug log
Test running one or two local diode_node servers, connect the CLI to them and then kill one of them to reproduce what Hans has been seeing.