This repository was archived by the owner on Mar 1, 2026. It is now read-only.
Issue #904: Crash in rocksdb::IOStatsContext::Reset, this=NULL#905
Open
spetrunia wants to merge 1 commit into
Open
Issue #904: Crash in rocksdb::IOStatsContext::Reset, this=NULL#905spetrunia wants to merge 1 commit into
spetrunia wants to merge 1 commit into
Conversation
Fix both code paths: - Change the test source code so it doesn't cause the "Unused variable" warning (which -Werror converted into error and caused CMake not to set HAVE_THREAD_LOCAL) - If the system doesn't seem to support HAVE_THREAD_LOCAL, refuse to compile (rather than producing a binary that crashes for some tests)
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@hermanlee has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Contributor
|
C++11 thread_local seems to be fully supported starting with GCC 4.8, clang 3.3, MSVC 2015 - maybe that's good enough to switch to thread_local and drop CMake checks altogether? |
hermanlee
approved these changes
Nov 21, 2018
Contributor
|
Removing the check should be okay too. The check looks to be inherited from RocksDB cmake files. @spetrunia, I assume none of your builds actually fail with your updated cmake check and thread local is supported across all builds? |
svoj
pushed a commit
to MariaDB/server
that referenced
this pull request
May 16, 2019
Fix both code paths: - Change the test source code so it doesn't cause the "Unused variable" warning (which -Werror converted into error and caused CMake not to set HAVE_THREAD_LOCAL) - If the system doesn't seem to support HAVE_THREAD_LOCAL, refuse to compile (rather than producing a binary that crashes for some tests) Originally submitted at facebook/mysql-5.6#905
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix both code paths:
Change the test source code so it doesn't cause the "Unused variable"
warning (which -Werror converted into error and caused CMake not to set
HAVE_THREAD_LOCAL)
If the system doesn't seem to support HAVE_THREAD_LOCAL, refuse to
compile (rather than producing a binary that crashes for some tests)