Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Incorrect enumeration behavior #1881

Closed
jherico opened this issue Apr 17, 2022 · 1 comment · Fixed by #1905
Closed

Incorrect enumeration behavior #1881

jherico opened this issue Apr 17, 2022 · 1 comment · Fixed by #1905

Comments

@jherico
Copy link

jherico commented Apr 17, 2022

let properties: Vec<ash::vk::ExtensionProperties> = unsafe {
let mut num = 0;
check_errors(fns.v1_0.enumerate_instance_extension_properties(
ptr::null(),
&mut num,
ptr::null_mut(),
))?;
let mut properties = Vec::with_capacity(num as usize);
check_errors(fns.v1_0.enumerate_instance_extension_properties(
ptr::null(),
&mut num,
properties.as_mut_ptr(),
))?;
properties.set_len(num as usize);
properties
};

The implementation of an "enumerate" function as a simple "fetch the number, then populate the results with that number" is a common mistake in Vulkan code.

While rare, code must deal with possibility that the number returned by the first call won't be valid by the time of the second call, and that the second call will have populated the num value with a different number than the original (in this case, the function should also have returned VK_INCOMPLETE)

Consider the implementation of the C++ bindings vk::enumerateInstanceExtensionProperties method:

    do
    {
      result = static_cast<Result>( d.vkEnumerateInstanceExtensionProperties( layerName ? layerName->c_str() : nullptr, &propertyCount, nullptr ) );
      if ( ( result == Result::eSuccess ) && propertyCount )
      {
        properties.resize( propertyCount );
        result = static_cast<Result>( d.vkEnumerateInstanceExtensionProperties( layerName ? layerName->c_str() : nullptr, &propertyCount, reinterpret_cast<VkExtensionProperties*>( properties.data() ) ) );
      }
    } while ( result == Result::eIncomplete );

This creates a loop that will almost invariably exit after the first pass, but which will still handle the case where the number of exposed extensions changes at runtime between the first and second calls. This same logic should be applied to all the other functions with potentially return VK_INCOMPLETE. Searching for Result::eIncomplete in this file gives a good list of them I believe.

@Rua
Copy link
Contributor

Rua commented Apr 18, 2022

Huh, interesting. I'll look into implementing that sometime.

Rua added a commit to Rua/vulkano that referenced this issue May 31, 2022
@Rua Rua mentioned this issue May 31, 2022
AustinJ235 pushed a commit that referenced this issue May 31, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants