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

drain should not be called by multiple coroutines #114

Closed
komuw opened this issue May 29, 2019 · 1 comment · Fixed by #113
Closed

drain should not be called by multiple coroutines #114

komuw opened this issue May 29, 2019 · 1 comment · Fixed by #113
Labels
bug Something isn't working

Comments

@komuw
Copy link
Owner

komuw commented May 29, 2019

see: https://bugs.python.org/issue29930

naz/naz/client.py

Line 1312 in 5ebeffe

await self.writer.drain()

We need to acquire an asyncio.Lock around that code.

@komuw komuw added the bug Something isn't working label May 29, 2019
@komuw
Copy link
Owner Author

komuw commented May 29, 2019

I think we also need to acquire an asyncio.Lock for the code to re-establish connection:

naz/naz/client.py

Line 1206 in ae4bae4

async def re_establish_conn_bind(self, smpp_command: str, log_id: str) -> None:

@komuw komuw mentioned this issue May 31, 2019
komuw added a commit that referenced this issue May 31, 2019
What:
- Introduce a configurable timeout when trying to connect to SMSC
- Try and detect when the connection to SMSC is disconnected and attempt to re-connect & re-bind
- bugfix; `asyncio.streams.StreamWriter.drain` should not be called concurrently by multiple coroutines[1]
- when shutting down, `naz` now tries to make sure that write buffers are properly flushed[2][3]

Why:
- Make `naz` more failure tolerant
- Fixes: #67
- Fixes: #114
- Fixes: #116
- Fixes: #117
- Fixes: #120

References:
1. https://bugs.python.org/issue29930
2. https://vorpus.org/blog/some-thoughts-on-asynchronous-api-design-in-a-post-asyncawait-world/
3. aio-libs/aiohttp#1369
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant