[SERVER-36400] Explicitly destroy the client on exiting the run body of each BackgroundJob Created: 01/Aug/18 Updated: 29/Oct/23 Resolved: 06/Aug/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Storage |
| Affects Version/s: | None |
| Fix Version/s: | 4.0.3, 4.1.2 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Xiangyu Yao (Inactive) | Assignee: | Xiangyu Yao (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | neweng | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||
| Sprint: | Storage NYC 2018-08-13 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Linked BF Score: | 3 | ||||||||||||||||||||
| Description |
|
We set the state to be "Done" before the thread_local client gets destroyed because the thread is still running. But setting the state to be "Done" and notifying would unblock the main thread which could go all the way to the destructor of ServiceContext. Therefore, we could have a situation where the client of WTCheckpointThread gets destroyed by its thread after ServiceContext gets destroyed by main thread. The way to reproduce BF-10032 is adding a big sleep here. The fix should be similar to We should check other BackgroundJobs which create clients in their run() function. |
| Comments |
| Comment by Githook User [ 13/Sep/18 ] | ||||||||||||
|
Author: {'name': 'Henrik Edin', 'email': 'henrik.edin@mongodb.com', 'username': 'henrikedin'}Message: This patch does several loosely related and surprisingly hard to separate things. 1.) Make the ServiceContext class final 2.) Create a mechanism, called ConstructorActions, for running methods on 3.) Introduce / improve test fixture base classes for tests, giving them fresh 4.) Make several remaining global variables SC decorations in support of (3) 5.) Replace many MONGO_INITIALIZERS that access getGlobalServiceContext with the 6.) Fix up tests to use the fixtures from (3) and fix tests that silently used 7.) Utilize (2) as necessary to simplify initialization of new ServiceContexts, (cherry picked from commit d520be0814492c262515cf0a5d62a127ace70dce)
(cherry picked from commit 9a68eb0cc65a93233b4ff5746330f9eb77de9b90)
(cherry picked from commit bfe170e49b1dc10b2badde45bc13c057a2f8ab61)
(cherry picked from commit b079e4713d897b5541c2804386025817ec720800)
(cherry picked from commit 4c16f0f336f4db77034e8aa594bbd4a5bac3f40c)
(cherry picked from commit c9d4204b6243e5eee6fe0b5e2c34d02af9ac5edb) | ||||||||||||
| Comment by Githook User [ 06/Aug/18 ] | ||||||||||||
|
Author: {'username': 'xy24', 'name': 'Xiangyu Yao', 'email': 'xiangyu.yao@mongodb.com'}Message: | ||||||||||||
| Comment by Xiangyu Yao (Inactive) [ 06/Aug/18 ] | ||||||||||||
|
I've created | ||||||||||||
| Comment by Andy Schwerin [ 06/Aug/18 ] | ||||||||||||
|
xiangyu.yao, yeah, I think that's probably the best choice. I'd like to see if we can make Client::initThread private / accessible only to ScopedClient, and maybe get rid of Client::initThreadIfNotAlready, to further improve the situation, but maybe that's a longer term improvement. People do a lot of copy-paste / coding-by-example, so eliminating the bad pattern would be ideal, if it's not more than a few hours work. | ||||||||||||
| Comment by Xiangyu Yao (Inactive) [ 06/Aug/18 ] | ||||||||||||
|
schwerin Is it something like:
| ||||||||||||
| Comment by Andy Schwerin [ 06/Aug/18 ] | ||||||||||||
|
Perhaps it's time to make a dedicated RAII class to manage Client lifetime on threads. I have ideas. |