Race condition in shader object layer?

[qbojj]

In UpdateDrawState CommandBufferData& data is guaranteed to be externally-synchronized and it owns the FullDrawStateData *state_data.

What worries me is that Shader *vertex_or_mesh_shader taken from state_data->GetShader(...) is not and Shader::pipelines does not use mutexes. I haven't found anything that makes accesses to different Shaders synchronized.

So is it a race condition or is access to VkShaderEXT (being bound to a command buffer) externally-synchronized or something else?

Vulkan-ExtensionLayer/layers/shader_object.cpp

Lines 2436 to 2442 in ace98ff

	auto state_data_key = state_data->GetKey();
	auto found_pipeline_ptr = vertex_or_mesh_shader->pipelines.GetOrNullptr(state_data_key);
	VkPipeline pipeline = found_pipeline_ptr ? *found_pipeline_ptr : VK_NULL_HANDLE;
	if (pipeline == VK_NULL_HANDLE) {
	pipeline = CreateGraphicsPipelineForCommandBufferState(data);
	vertex_or_mesh_shader->pipelines.Add(state_data_key, pipeline);
	}

[ziga-lunarg]

You are correct, this is a bug. Thank you for reporting the issue

[jeremyg-lunarg]

Reopening because @qbojj wrote the following:

There's still a problem.
If a pipeline was created in another thread during locking of data for writing, that pipeline should be bound, but in current implementation VK_NULL_HANDLE is bound.

Vulkan-ExtensionLayer/layers/shader_object.cpp

Lines 2488 to 2498 in 6282367

	if (pipeline == VK_NULL_HANDLE) {
	std::unique_lock<std::shared_mutex> lock;
	auto& pipelines = vertex_or_mesh_shader->pipelines.GetDataForWriting(lock);
	// Ensure that a pipeline for this state wasn't created in another thread between the read lock above and the write lock
	if (pipelines.NumEntries() == pipeline_count || pipelines.Find(state_data_key) == pipelines.end()) {
	pipeline = CreateGraphicsPipelineForCommandBufferState(data);
	pipelines.Add(state_data_key, pipeline);
	}
	}
	data.device_data->vtable.CmdBindPipeline(commandBuffer, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline);

[jeremyg-lunarg]

nevermind, the new problem is now in #282