[SERVER-47124] Enforce initializer order for MONGO_REGISTER_TEST_COMMAND Created: 25/Mar/20 Updated: 29/Oct/23 Resolved: 25/Jun/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 4.7.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Sara Golemon | Assignee: | Gabriel Marks |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | neweng | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Operating System: | ALL |
| Sprint: | Security 2020-06-29 |
| Participants: |
| Description |
|
The helper macro MONGO_REGISTER_TEST_COMMAND currently requests an initializer be run "whenever", that is, without prependents or dependents. Fortunately, as an implementation detail, our initializer graph interprets this as "after all the complex initializers are done. However, our implementation could just as validly do the opposite and be equally correct. In reality, we need these registrants to run after the `--setParameter EnableTestCommands=1` flag has been evaluated otherwise we risk test-only commands not being available when they should be if the implementation ever changes. Add a prerequisite to this macro to run after "EndStartupOptionHandling" |
| Comments |
| Comment by Sara Golemon [ 08/Dec/20 ] |
|
Having seen Go ahead and revert this if it simplifies things. As mentioned, I agree with you that it's not necessary given the role of "default" actually sitting somewhere in the middle of the execution order rather than my prior understanding. |
| Comment by Billy Donahue [ 07/Dec/20 ] |
|
It's the only place that uses the ::mongo::defaultInitializerName() outside of init itself, so I was thinking of just resimplifying this thing down so I can get rid of ::mongo::defaultInitializerName() altogether. I'm also adding a fair amount of documentation about the startup phases so this will not be so mysterious anymore. |
| Comment by Sara Golemon [ 07/Dec/20 ] |
|
You're correct. I had misread this loop as happening after "default" (which all the test commands are as well) which would have been incorrect. You're right that this is a no-op and could be reverted to save a small amount of startup work. I disagree that this change hurts code clarity though. This change makes it more apparent that the TEST_COMMAND initializers are running after StartupOptionHandling, and have no impact to readability elsewhere. |
| Comment by Billy Donahue [ 07/Dec/20 ] |
|
This didn't fix any actual problem. I am recommending that we revert it as it is a downgrade in code clarity for the MONGO_REGISTER_TEST_COMMAND macro. The premise of the ticket was flawed. The MONGO_INITIALIZER macro does not mean "run whenever". There's a sequence point called "default" that will automatically be a prerequisite of any MONGO_INITIALIZER. Depending on "default" and "EndStartupOptionHandling" is redundant. Begin and End "StartupOptionHandling" are defined by util/options_parser/startup_options_init.cpp All the rules in that file happen after "ValidateLocale" and before "default", so this change really was a noop. |
| Comment by Githook User [ 25/Jun/20 ] |
|
Author: {'name': 'Gabriel Marks', 'email': 'gabriel.marks@mongodb.com', 'username': 'marksg07'}Message: |