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

Thread-safety and locking is changing #109

Open
moggers87 opened this issue Jul 23, 2019 · 3 comments
Open

Thread-safety and locking is changing #109

moggers87 opened this issue Jul 23, 2019 · 3 comments
Labels
Milestone

Comments

@moggers87
Copy link
Owner

Currently Salmon will lock every call to a handler by default. This is great for thread safety, but absolutely garbage for performance. There is a nolocking decorator if you know your code is thread-safe.

I was wondering how current users of Salmon would feel if I reversed this situation and made Salmon assume handlers were thread-safe?

Given that web frameworks (e..g Django and Flask) expect users to write thread-safe code, I don't think this is a huge ask. For my own purposes, I already write thread-safe code and I'm always using nolocking.

If I made this change, that would mean I'd have to go through various bits of Salmon code and make sure they're thread-safe (e.g. Queue is definately not thread-safe).

What does everyone else think?

@moggers87 moggers87 added this to the 4.0 milestone Jul 23, 2019
@moggers87
Copy link
Owner Author

This would be a 4.0 change if I do decide to do it, so don't worry 😄

@moggers87 moggers87 pinned this issue Aug 10, 2019
@moggers87 moggers87 changed the title Thread-safety and locking Thread-safety and locking question Aug 10, 2019
@moggers87 moggers87 changed the title Thread-safety and locking question Thread-safety and locking is changing May 27, 2020
@moggers87
Copy link
Owner Author

Also worth noting that this would require the FSM to be thread-safe too. I think this is (accidentally) already the case with the default FSM.

@DrDub
Copy link

DrDub commented Dec 5, 2021

This change makes a lot of sense. Look forward to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants