[Freeciv-Dev] Re: another cma-crash (PR#1888)

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Wed, Aug 07, 2002 at 12:53:06AM +0300, Jussi Asp wrote:
> Hi,
> 
> Here's an another crash it got today, with latest cvs-version.
> included a savegame which caused the crash. join as haakon vii and ivan 
> the terrible and take haakon empty city with ivan's unit. city had max 
> food cma on.
> I hope you find the bug, using cma is so much more fun than without it. :)

Analysis:

new owner                               server                                  
     old owner
 - client sends move request
                                        - server sends a city_info packet with 
an
                                          updated city_map (because of the 
enemy 
                                          unit)
                                        - server transfers city to new owner
                                        - server sends a city_info packet with 
the 
                                          new owner to the old owner
                                                                                
    - receives the first city_info packet
                                                                                
    - handle_city is called
                                                                                
    - CMA calculates
                                                                                
    - CMA want to apply the new allocation 
                                                                                
      at the server
                                                                                
    - sends the changes to the server
                                                                                
    - wait for the results
                                                                                
      - receives the second city_info packet
                                                                                
      - city has changed owner in the 
                                                                                
        deep guts of CMA
                                                                                
      - KABOOM

The patch add two new packets which act as a general wrappers around a
set of packets. So the reports and agents are only updated if
PACKET_OTHER_PROCESSING_FINISHED (POPF) is received. With the patch it
works like this (also added are PACKET_PROCESSING_STARTED (PPS) and
PACKET_PROCESSING_FINISHED (PPF):

new owner                               server                                  
     old owner
 - client sends move request
                                        - start serving the request:
                                          - server sends PPS to the new owner
                                          - server sends POPS to the old owner
                                        - server sends a city_info packet with 
an
                                          updated city_map (because of the enemy
                                          unit)
                                        - server transfers city to new owner
                                        - server sends a city_info packet with 
the
                                          new owner to the old owner
                                        - the server sends a city_info packet 
with the
                                          new owner to the old owner
                                        - the execution of the request is 
finished:
                                          - PPF to the new owner
                                          - POPF to the old owner
                                                                                
    - receives POPS; freeze
                                                                                
    - receives the first city_info packet
                                                                                
    - receives the second city_info packet
                                                                                
    - receives POPF; thaw
                                                                                
    - handle_city is called
                                                                                
    - CMA sees that the city is no longer 
                                                                                
      owned and detaches itself

POPS and POPF provide a general and neutral (the PPS and PPF change
state because they are associated with requests) to freeze/thaw the
client. Currently this is used for reports (because for speed) and
agents (because of inconsistent data). But it can be extended to
include map drawing and so on.

Mainly to Vasco: It is also possible to add
PLEASE_SEND_ME_POPS_IF_YOU_SEE_THIS_PACKET and
PLEASE_SEND_ME_POPF_IF_YOU_SEE_THIS_PACKET and so rewrite:
  while doing stuff:
    last_request=send_some_requests
  reports_freeze_till(last_request)
to
  send_empty(PLEASE_SEND_ME_POPS_IF_YOU_SEE_THIS_PACKET)
  while doing stuff:
    send_some_requests
  send_empty(PLEASE_SEND_ME_POPF_IF_YOU_SEE_THIS_PACKET)

IMHO this isn't an improvement because the current isn't complicated.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Wenn ihr nicht sofort verschwindet, machen wir euch fertig." 
 "Wer ist wir? Du bist doch ganz allein?" "W6, W6, W6 und Ich."
    -- Alessandro Macri in de.rec.spiele.rpg.misc

POPS1.diff
Description: Text document