[SERVER-19143] race in setting OpDebug ns can cause invalid BSON to be returned from currentOp command Created: 25/Jun/15 Updated: 09/Dec/16 Resolved: 29/Jun/15 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | 3.1.4 |
| Fix Version/s: | 3.1.5 |
| Type: | Bug | Priority: | Critical - P2 |
| Reporter: | Adam Midvidy | Assignee: | Adam Midvidy |
| Resolution: | Done | Votes: | 0 |
| Labels: | UT, dnsf | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||
| Backwards Compatibility: | Minor Change | ||||||||||||||||
| Operating System: | ALL | ||||||||||||||||
| Steps To Reproduce: | run evalc.js in a loop, or run basic or basicPlus.js |
||||||||||||||||
| Sprint: | Platform 5 06/26/16 | ||||||||||||||||
| Participants: | |||||||||||||||||
| Linked BF Score: | 0 | ||||||||||||||||
| Description |
|
The currentOp command calls CurOp::reportState to report the state of an operation. CurOp::reportState will fill in the "ns" field of its passed BSONObjBuilder with it's _ns field OR the ns field of its OpDebug member if _ns is not set.
Unfortunately, access to the OpDebug field is not synchronized. As such, the "ns" field of an operation object returned in the currentOp command can contain garbage data. When the garbage data does not end with a null byte, this can resullt in invalid BSON being returned to the client. As part of OP_COMMAND work, the shell will now validate BSON in command responses from the server, which uncovered this issue. For example - here are the raw bytes from a problematic currentOp response:
Problematic part (offsets 148 through 155)
at offset 154, the content of an 'ns' field has a 't' (0x74) instead of a trailing null before the start of the next element. |
| Comments |
| Comment by Adam Midvidy [ 29/Jun/15 ] | |||||||||||||||||
|
The fix for this issue has resulted in a minor behavior change in the profiling output for commands that have the _ns field of their associated CurOp set when acquiring locks. Profiling output for an 'distinct' command (with patch):
Profiling output for an 'distinct' command (without patch):
Note the change in the 'ns' field. | |||||||||||||||||
| Comment by Githook User [ 29/Jun/15 ] | |||||||||||||||||
|
Author: {u'username': u'amidvidy', u'name': u'Adam Midvidy', u'email': u'amidvidy@gmail.com'}Message: | |||||||||||||||||
| Comment by Githook User [ 26/Jun/15 ] | |||||||||||||||||
|
Author: {u'username': u'kaloianm', u'name': u'Kaloian Manassiev', u'email': u'kaloian.manassiev@mongodb.com'}Message: Revert " This reverts commit 780745072104ee4d0514f09e71765ac99d380bdc. | |||||||||||||||||
| Comment by Githook User [ 26/Jun/15 ] | |||||||||||||||||
|
Author: {u'username': u'amidvidy', u'name': u'Adam Midvidy', u'email': u'amidvidy@gmail.com'}Message: |