Opened 2 years ago
Last modified 4 weeks ago
#8544 new defect (bug)
Messages sent at the same time can encounter a race condition where they share the same thread_id
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Up Next | Priority: | normal |
Severity: | normal | Version: | 1.0 |
Component: | Messages | Keywords: | has-patch |
Cc: |
Description
buddypress/bp-messages/classes/class-bp-messages-message.php
send() method
When sending a new message the thread id is fetched using
SELECT MAX(thread_id) FROM wp_bp_messages_messages
However if two messages are sent at the same time it's possible for both messages to get assigned the same thread_id.
It would be better to keep a separate table of threads with unique ids which are auto-incremented.
When creating a new thread, a row is inserted in to this table and the mysql_insert_id() command will return the correct ID.
Attachments (1)
Change History (8)
#1
@
2 years ago
- Component changed from Core to Messages
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Awaiting Contributions
- Version changed from 9.0.0 to 1.0
@
2 years ago
Patch to ensure thread_id is generated in a single db transaction while inserting a new message
#2
@
2 years ago
- Keywords has-patch added; needs-patch removed
- Milestone changed from Awaiting Contributions to Awaiting Review
I've created a patch which will ensure that the thread_id is generated at the same time as the message is inserted, in a single transaction.
This will ensure that two messages submitted at the same time cannot end up being assigned the same thread_id.
The generated thread_id is then fetched based on the newly inserted message id.
#3
@
2 years ago
- Milestone changed from Awaiting Review to 10.0.0
Thanks a lot for your patch! I just gave a quick look and it's very promising. I'll review it asap and we'll work on having this fix available in 10.0.0.
#4
@
22 months ago
Is it possible to have unit tests to replicate this issue or to show it works as desired?
Hi @petersmithrestless
Thanks for your feedback. I agree it would be better to have a different table. But I believe improving this would be a pretty massive change to deal with. I'd be happy to have a patch about it making sure to take care back compatibility.