CodingCaius opened a new issue, #61:
URL: https://github.com/apache/casbin-redis-watcher/issues/61
### Description
Calling `Close()` on one Redis watcher publishes the plain-text payload
`"Close"` to the shared policy channel. Every other watcher on that channel
attempts to decode it as a JSON `MSG`, resulting in:
```text
Failed to parse message: Close with error: invalid character 'C' looking for
beginning of value
96 <nil>
```
This occurs during normal shutdown in a multi-instance deployment. It does
not require a custom callback or message format. Both the shutdown publisher
and JSON decoder are inside this library.
### Versions
- Published module: `github.com/casbin/redis-watcher/v2 v2.8.0`
- Also reproduced on current `master`:
`b23b3433cd156f26081beb6c0e743ac966c094f9`
- Redis 7.2, standalone
### Reproduction
1. Create two watchers with the same Redis address and
`WatcherOptions.Channel`.
2. Wait until both subscriptions are registered.
3. Call `Close()` on the first watcher.
4. The second watcher logs the JSON decoding error above. An independent
Redis subscriber also observes the literal payload `Close`.
5. The second watcher can still receive subsequent valid policy updates; the
observed problem is the invalid shutdown broadcast, not loss of all subsequent
updates.
I have a regression test using a third, independent Redis subscriber. It
fails on unmodified master with:
```text
--- FAIL: TestWatcherCloseDoesNotPublish
Close published a non-JSON message: "Close"
```
### Expected behavior
Closing one watcher should stop its local subscription and release its owned
resources without publishing a shutdown message to other watchers.
### Cause and proposed fix
`Close()` closes the local `w.close` channel and then publishes `"Close"`.
The receiving goroutine only checks `w.close` after receiving from `for msg :=
range ch`, so the publish currently wakes the local receiver. Other instances
instead try to unmarshal that same payload as JSON.
The receive loop can select on both `w.close` and the Redis message channel,
allowing local cancellation to wake it directly. The existing deferred cleanup
can then release the subscription and clients without the shutdown publish.
Making repeated `Close()` calls safe would also avoid closing an already-closed
signal channel.
Related historical discussion: #20. That issue concerned channel usage and
was closed after clarification; this report describes the reproducible
cross-instance decoding error.
I would like to submit a small fix with regression tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
