Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition in MQService initialization #637

Closed
cleborys opened this issue Jul 31, 2020 · 1 comment · Fixed by #638
Closed

Race condition in MQService initialization #637

cleborys opened this issue Jul 31, 2020 · 1 comment · Fixed by #638
Assignees
Labels

Comments

@cleborys
Copy link
Member

Looks like there is a pesky concurrency issue in the message service:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/asyncio/runners.py", line 43, in run
    return loop.run_until_complete(main)
  File "/usr/local/lib/python3.7/asyncio/base_events.py", line 587, in run_until_complete
    return future.result()
  File "server.py", line 73, in main
    service.initialize() for service in services.values()
  File "/code/server/game_service.py", line 63, in initialize
    await self._message_queue_service.declare_exchange("faf-rabbitmq")
  File "/code/server/message_queue_service.py", line 80, in declare_exchange
    new_exchange = await self._channel.declare_exchange(
AttributeError: 'NoneType' object has no attribute 'declare_exchange'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/aiormq/connection.py", line 374, in __reader
    weight, channel, frame = await self.__receive_frame()
  File "/usr/local/lib/python3.7/site-packages/aiormq/connection.py", line 326, in __receive_frame
    frame_header = await self.reader.readexactly(1)
  File "/usr/local/lib/python3.7/asyncio/streams.py", line 679, in readexactly
    await self._wait_for_data('readexactly')
  File "/usr/local/lib/python3.7/asyncio/streams.py", line 473, in _wait_for_data
    await self._waiter
concurrent.futures._base.CancelledError
  1. Game service initialize and message queue service initialize are both started and awaited with asyncio.gather
  2. message queue service successfully connects and yields on _connection.channel
  3. Game service calls declare_exchange

Now the message queue service is in a state where _connection is initialized, but _channel is still None

@cleborys cleborys self-assigned this Jul 31, 2020
@cleborys cleborys added the bug label Jul 31, 2020
@cleborys
Copy link
Member Author

The cleanest way to prevent this that I can think of is to add an explicit is_connected bool to the service to replace all the None checks for _connection and make declare_exchange wait for a short timeout if is_connected is not set.

On the other hand, we are not likely to ever have more than one exchange, so one could also move that part into the initialization of the message service. But we would probably have a similar race condition as soon as we allow other services to listen on the message service, hence I think adding a better is_connected check is preferable.

Askaholic pushed a commit that referenced this issue Sep 3, 2020
* Add failing integration test for race condition

* Add service._is_ready checks

* Add initialization_lock to message_queue_service

* Reformat message_queue_service

* replace success flag of _connect with Exception

* Review comments

* declare_exchange triggers initialization if necessary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant