Race condition fix; caused latency on first load

The issue was that the user could open a C-family file and have it start
compiling in the background. While it is still compiling, he could trigger the
completion system with a member dot operator. Because the file was still
compiling for the very first time, the TranslationUnit object was yet not
created. Sadly, this meant that UpdatingTranslationUnit would return false, and
a new query request would be created, the GUI would hang until it was done
aaaaand terrible lag until the file was compiled.

This was a very rare edge case that could also only be triggered if it takes a
considerable amount of time to compile the file.
This commit is contained in:
Strahinja Val Markovic 2013-01-24 18:37:44 -08:00
parent a2343cad4c
commit 8cc9c9ca76
3 changed files with 31 additions and 0 deletions

View File

@ -140,6 +140,9 @@ bool ClangCompleter::UpdatingTranslationUnit( const std::string &filename ) {
if ( !unit ) if ( !unit )
return false; return false;
// Thankfully, an invalid, sentinel TU always returns true for
// IsCurrentlyUpdating, so no caller will try to rely on the TU object, even
// if unit is currently pointing to a sentinel.
return unit->IsCurrentlyUpdating(); return unit->IsCurrentlyUpdating();
} }
@ -366,14 +369,23 @@ shared_ptr< TranslationUnit > ClangCompleter::GetTranslationUnitForFile(
translation_unit_created = false; translation_unit_created = false;
return it->second; return it->second;
} }
// We create and store an invalid, sentinel TU so that other threads don't
// try to create a TU for the same file while we are trying to create this
// TU object. When we are done creating the TU, we will overwrite this value
// with the valid object.
filename_to_translation_unit_[ filename ] =
make_shared< TranslationUnit >();
} }
shared_ptr< TranslationUnit > unit; shared_ptr< TranslationUnit > unit;
try { try {
unit = make_shared< TranslationUnit >( unit = make_shared< TranslationUnit >(
filename, unsaved_files, flags, clang_index_ ); filename, unsaved_files, flags, clang_index_ );
} catch ( ClangParseError & ) { } catch ( ClangParseError & ) {
Erase( filename_to_translation_unit_, filename );
return unit; return unit;
} }

View File

@ -36,6 +36,11 @@ namespace YouCompleteMe {
typedef shared_ptr < typedef shared_ptr <
remove_pointer< CXCodeCompleteResults >::type > CodeCompleteResultsWrap; remove_pointer< CXCodeCompleteResults >::type > CodeCompleteResultsWrap;
TranslationUnit::TranslationUnit()
: filename_(""),
clang_translation_unit_( NULL ) {
}
TranslationUnit::TranslationUnit( TranslationUnit::TranslationUnit(
const std::string &filename, const std::string &filename,
const std::vector< UnsavedFile > &unsaved_files, const std::vector< UnsavedFile > &unsaved_files,
@ -85,6 +90,10 @@ void TranslationUnit::Destroy() {
std::vector< Diagnostic > TranslationUnit::LatestDiagnostics() { std::vector< Diagnostic > TranslationUnit::LatestDiagnostics() {
std::vector< Diagnostic > diagnostics; std::vector< Diagnostic > diagnostics;
if ( !clang_translation_unit_ )
return diagnostics;
unique_lock< mutex > lock( diagnostics_mutex_ ); unique_lock< mutex > lock( diagnostics_mutex_ );
// We don't need the latest diags after we return them once so we swap the // We don't need the latest diags after we return them once so we swap the
@ -103,6 +112,11 @@ std::vector< Diagnostic > TranslationUnit::LatestDiagnostics() {
bool TranslationUnit::IsCurrentlyUpdating() const { bool TranslationUnit::IsCurrentlyUpdating() const {
// We return true when the TU is invalid; an invalid TU also acts a sentinel,
// preventing other threads from trying to use it.
if ( !clang_translation_unit_ )
return true;
unique_lock< mutex > lock( clang_access_mutex_, try_to_lock_t() ); unique_lock< mutex > lock( clang_access_mutex_, try_to_lock_t() );
return !lock.owns_lock(); return !lock.owns_lock();
} }

View File

@ -41,6 +41,11 @@ typedef boost::shared_ptr< std::vector< CompletionData > > AsyncCompletions;
class TranslationUnit : boost::noncopyable { class TranslationUnit : boost::noncopyable {
public: public:
// This constructor creates an invalid, sentinel TU. All of it's methods
// return empty vectors, and IsCurrentlyUpdating always returns true so that
// no callers try to rely on the invalid TU.
TranslationUnit();
TranslationUnit( TranslationUnit(
const std::string &filename, const std::string &filename,
const std::vector< UnsavedFile > &unsaved_files, const std::vector< UnsavedFile > &unsaved_files,