On Sun, 1 May 2022 09:33:19 +0000 Stian Grenborgen <stian...@student.matnat.uio.no> wrote: > I'm wondering why we are using Connection.request instead of just > Connection.sendMessage.
Almost certainly its historic. When the DOM-removal was happening we went
to really conservative messaging (e.g. overuse of askMessage), which slowed
the game down but made it nearly synchronous and deterministic, and thus
easy to debug. When that work was complete things were loosened up a bit
(e.g. calling handlers in their own thread). I doubt we were coherent or
consistent about this, but it did get to the point that the message
handling was both working faster and easy to debug now that it was logged
if required. At that point, profiling showed all the lag was in the GUI,
and I was buried by non-FreeCol work, so messaging was ignored.
> The point of having Connection.askMessage is connecting the reply to the
> answer ... so that the calling thread can be blocked until the answer has
> been received. It's for making an asynchronous process synchronous.
>
> Yet the process is made asynchronous again by calling the handler in
> Connection.request.
>
> The main difference between Connection.request and Connection.sendMessage is
> that the caller to the former gets blocked until the reply has arrived, while
> for the latter it does not. Please note that this only happens for the
> calling thread ... there's nothing preventing another thread from making
> another request at the same time. The reply we are waiting for can already
> have been invalidated. The value of this type of locking is dubious.
This was again probably just over-caution. No one was in a rush to deal
with a bunch of race conditions, like happened the time I tried making
moveMessage not have to wait:-). That said, there is a lot of code in the
client InGameController which reads:
if (askServerToDoSomething(...)) {
if (looksLikeTheServerDidIt(...)) {
doStuffThatDependsOnTheChange()
...where the first test sometimes just succeeds if the client
sent the message (ServerAPI.send), but often waits for a server
response usually containing an UpdateMessage that provides the requested
change (ServerAPI.ask). So, point taken about the dubious locking, but
the client code tends to explicitly enforce the required sequential
behaviour.
Now, I expect there are individual cases where asks can be sends, but we
would have to look hard at each case. Notably, as you say, the most common
action (move) is probably a bad candidate.
> In addition, any time it is used we need to consider when to abort the
> execution and if a timeout should be added to the call.
There are certainly timeout cases already, needed to avoid indefinite
stalls in multiplayer. See the various uses of TimedSession. There are
probably more cases on the server side where a more asychronous style
would work.
> There are two approaches we can use instead of the current solution:
>
> 1. Have a completely asynchronous process when sending messages.
> 2. Execute Connection.request/askMessage on separate threads (either
> popup-threads or have a thread poll/ExecutorService).
Whatever is easier to maintain:-). My main concern is that we have cases
that *must* block, so a synchronous layer will be needed on top.
> I am also noticing that we are calling Connection.request from the Event
> Dispatching Thread. This makes the user interface block until the answer has
> been received. The Connection.askMessage, and by extension
> Connection.request, should only be called from worker threads.
Yep, I have historically been pretty casual whether we are in the event
thread or not. Not good practice I know. Where are you seeing this?
I just looked at what I thought would be a likely case, but the emigration
handling looks good as the server messaging happens in its own thread
inside DialogCallback.
Cheers,
Mike Pope
pgp4cuqUp0uuu.pgp
Description: OpenPGP digital signature
_______________________________________________ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers
