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

SERVER-25628 make FileAllocator::get() thread-safe #1107

Closed
wants to merge 1 commit into from

Conversation

yjhjstz
Copy link

@yjhjstz yjhjstz commented Aug 16, 2016

No description provided.

@@ -472,11 +472,9 @@ void FileAllocator::run(FileAllocator* fa) {
}
}

FileAllocator* FileAllocator::_instance = 0;
FileAllocator* FileAllocator::_instance = new FileAllocator();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this approach, what about removing the _instance static member from FileAllocator, and instead making a static instance of FileAllocator inside of FileAllocator::get().

That is,

FileAllocator* FileAllocator::get() {
    static FileAllocator instance;
    return &instance;
}

Now that our minimum supported compilers provide thread-safe function static initialization, we can use this approach and ensure that the file allocator is allocated on the first call to get() instead of during static initialization. This is important, in case get() is never called or if the file allocator constructor some day depends on items that are constructed during or after static initialization.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's better!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind putting up a pull request to that effect, and I'll take care of merging it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have modified it by git commit --amend.

@amschwerin
Copy link
Contributor

Great. I'm running it through our test suite, and will merge afterward.

@amschwerin
Copy link
Contributor

Committed as 4debb62! Thanks!

@amschwerin amschwerin closed this Aug 23, 2016
@yjhjstz yjhjstz deleted the fix-filealloctor branch August 24, 2016 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants