Skip to content

Commit

Permalink
Serialize inclusions by the CountingIncluder
Browse files Browse the repository at this point in the history
Should fix ASAN failures in the CountingIncluderTest:
The ConcreteCountingIncluder include delegate is not safe
to call concurrently.  It's just simpler/nicer for the base
class to protect us in this way.
  • Loading branch information
dneto0 committed Sep 26, 2017
1 parent 6db3870 commit 9034a12
Showing 1 changed file with 19 additions and 5 deletions.
24 changes: 19 additions & 5 deletions libshaderc_util/include/libshaderc_util/counting_includer.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@

#include "glslang/Public/ShaderLang.h"

#include "libshaderc_util/mutex.h"

namespace shaderc_util {

// An Includer that counts how many #include directives it saw.
// Inclusions are internally serialized, but releasing a previous result
// can occur concurrently.
class CountingIncluder : public glslang::TShader::Includer {
public:
// Done as .store(0) instead of in the initializer list for the following
Expand All @@ -42,22 +46,28 @@ class CountingIncluder : public glslang::TShader::Includer {
// requesting source. For the semantics of the result, see the base class.
// Also increments num_include_directives and returns the results of
// include_delegate(filename). Subclasses should override include_delegate()
// instead of this method.
// instead of this method. Inclusions are serialized.
glslang::TShader::Includer::IncludeResult* includeSystem(
const char* requested_source, const char* requesting_source,
size_t include_depth) final {
++num_include_directives_;
return include_delegate(requested_source, requesting_source,
IncludeType::System, include_depth);
include_mutex_.lock();
auto result = include_delegate(requested_source, requesting_source,
IncludeType::System, include_depth);
include_mutex_.unlock();
return result;
}

// Like includeSystem, but for "local" include search.
glslang::TShader::Includer::IncludeResult* includeLocal(
const char* requested_source, const char* requesting_source,
size_t include_depth) final {
++num_include_directives_;
return include_delegate(requested_source, requesting_source,
IncludeType::Local, include_depth);
include_mutex_.lock();
auto result = include_delegate(requested_source, requesting_source,
IncludeType::Local, include_depth);
include_mutex_.unlock();
return result;
}

// Releases the given IncludeResult.
Expand All @@ -81,6 +91,10 @@ class CountingIncluder : public glslang::TShader::Includer {

// The number of #include directive encountered.
std::atomic_int num_include_directives_;

// A mutex to protect against concurrent inclusions. We can't trust
// our delegates to be safe for concurrent inclusions.
shaderc_util::mutex include_mutex_;
};
}

Expand Down

0 comments on commit 9034a12

Please sign in to comment.