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

asynchat.async_chat and asyncore.dispatcher_with_send are not thread-safe #70559

Closed
ngg mannequin opened this issue Feb 16, 2016 · 7 comments
Closed

asynchat.async_chat and asyncore.dispatcher_with_send are not thread-safe #70559

ngg mannequin opened this issue Feb 16, 2016 · 7 comments
Labels
stdlib Python modules in the Lib dir topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@ngg
Copy link
Mannequin

ngg mannequin commented Feb 16, 2016

BPO 26371
Nosy @gvanrossum, @vstinner, @1st1

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-02-16.20:58:05.927>
created_at = <Date 2016-02-16.20:47:24.872>
labels = ['invalid', 'type-bug', 'library', 'expert-asyncio']
title = 'asynchat.async_chat and asyncore.dispatcher_with_send are not thread-safe'
updated_at = <Date 2016-02-17.09:18:23.977>
user = 'https://bugs.python.org/ngg'

bugs.python.org fields:

activity = <Date 2016-02-17.09:18:23.977>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2016-02-16.20:58:05.927>
closer = 'gvanrossum'
components = ['Library (Lib)', 'asyncio']
creation = <Date 2016-02-16.20:47:24.872>
creator = 'ngg'
dependencies = []
files = []
hgrepos = []
issue_num = 26371
keywords = []
message_count = 7.0
messages = ['260367', '260368', '260369', '260380', '260381', '260383', '260384']
nosy_count = 4.0
nosy_names = ['gvanrossum', 'vstinner', 'yselivanov', 'ngg']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26371'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3', 'Python 3.4', 'Python 3.5', 'Python 3.6']

@ngg
Copy link
Mannequin Author

ngg mannequin commented Feb 16, 2016

The initiate_send() method in both asynchat.async_chat and asyncore.dispatcher_with_send is not a thread-safe function, but it can be called from multiple threads.

For example if asyncore.loop() runs in a background thread, and the main thread sends a message then both threads will call this.

It can result in sending (part of) the message multiple times or not sending part of it.

Possible solution:
asyncore.dispatcher_with_send.out_buffer and asynchat.async_chat.producer_fifo should be guarded with a lock (it should be locked even in writable())

@ngg ngg mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Feb 16, 2016
@vstinner
Copy link
Member

asynchat is now deprecated in favor of asyncio.

Almost all asyncio functions are not thread-safe, it's now well documented, see the general info:
https://docs.python.org/dev/library/asyncio-dev.html#concurrency-and-multithreading

If we do something, I suggest to only touch asynchat doc to explain well that asyncore and asynchat are not thread-safe.

@gvanrossum
Copy link
Member

I don't believe the original issue is a bug. There is nothing in the docs for asyncore or asynchat that suggests they would be thread-safe.

@ngg
Copy link
Mannequin Author

ngg mannequin commented Feb 17, 2016

If I want to write a TCP client which communicates back and forth with the server (both parties can send messages anytime) then it would be really easy to use it the following way:
Start a background thread with asyncore.loop(), and you can send messages easily, and handle_read() will be called automatically whenever data is received.
If this usage is not supported and I understand correctly then I can only send messages before starting the loop or callbacks from the loop (handle_accept, handle_read, etc).
This seems to be a much more difficult and error-prone way (at least for pre-Future python versions)

@vstinner
Copy link
Member

"If this usage is not supported (...)"

You can use threads, but access concurrently asyncore/asynchat from different threads. You have to build a communicate channel between your threads using thread-safe primitive like queue.Queue. asyncio has a builtin support for that: loop.call_soon_threadsafe().

Why not using asyncio instead of having to rebuild your own implementation?

@ngg
Copy link
Mannequin Author

ngg mannequin commented Feb 17, 2016

"Why not using asyncio instead of having to rebuild your own implementation?"

The issue happened with python 2.7 and we don't want that project to depend on additional libraries.
I think we will upgrade that project to python3 soon and will probably use asyncio then.

@vstinner
Copy link
Member

"I think we will upgrade that project to python3 soon and will probably use asyncio then."

Great :-) You may have a look at my https://pypi.python.org/pypi/sixer project, it may help :-)

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants