jfyne / live

Live views and components for golang
https://discord.gg/TuMNaXJMUG
MIT License
655 stars 27 forks source link

baseEngine.handleEmittedEvent not fully synced #68

Closed philippseith closed 1 year ago

philippseith commented 1 year ago

When more than one goroutine is calling socket.Self for the same socket, the page might not be rendered correctly. You can observe that in this sample: https://github.com/philippseith/gorx/blob/b1370a626b3b5018d700e55979a4cede273ace6a/ext/rxlive_test/rxlive_test.go#L18

I can see that baseEngine.handleSelf is synchronized, but not the rendering part in handleEmittedEvent.

When I synchronize the call to socket.Self (see here: https://github.com/philippseith/gorx/blob/957e88ddca118650351122095f80f681c5ea920f/ext/rxlive/viewmodel.go#L40), the page renders correctly.

IMHO all the steps executed in handleEmittedEvent for the socket should be synchronized, but just putting a mutex into this method would sync over all sockets, not only this one. Should there be a mutex in baseSocket.Self ?

jfyne commented 1 year ago

Hi, how do I observe? I run the test, go to http://localhost:8086. What should I be looking for?

jfyne commented 1 year ago

Ah figured it out

philippseith commented 1 year ago

If correctly synced, you will see one or two rows with Id AAA or BBB, where AAA sometimes vanishes. If sync does not work, lines will double, you might get more than one BBB row often, sometimes even more than one AAA row.

Mind that you need to use the https://github.com/philippseith/gorx/tree/feature/unsynchronized_sample branch, as I saw that my implementation of CombineLatest might cause reentrant calls to Observer[T].Next() and I fixed that in master meanwhile.

jfyne commented 1 year ago

Ok, try v0.15.6

jfyne commented 1 year ago

Nice catch, good luck with your project. Rxjs is an interesting beast