Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

layers: Fix checking if event is in use #9089

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

ziga-lunarg
Copy link
Contributor

Closes #8748

@ziga-lunarg ziga-lunarg requested a review from a team as a code owner December 24, 2024 21:55
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 331756.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 331771.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18437 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 331807.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18439 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18439 failed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 331979.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18440 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18440 failed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 332061.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18442 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 332098.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18444 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18444 failed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 332217.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18446 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18446 failed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 332246.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18447 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18447 passed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 333281.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18459 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18459 passed.

@@ -3775,6 +3789,15 @@ TEST_F(NegativeSyncObject, DISABLED_WaitEventThenSet) {
m_errorMonitor->SetDesiredError("VUID-vkSetEvent-event-09543");
vk::SetEvent(device(), event.handle());
m_errorMonitor->VerifyFound();

vkt::CommandPool second_pool(*m_device, m_second_queue->family_index);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mechanism to resolve wait form a different queue works for semaphores but not for events (events communicate within a single queue). I don't know a legal mechanism to signal event after the wait, it should be ordered (did not think about this when wrote initial version of the test). I think it's correct behavior that submission hangs in this scenario. One option is to have this test disabled by default only for manual inspection. Not sure if it causes timeout, if yes, we could rely on it in the test, even though that's not great.

Not sure why originally write_in_use was introduced. It was in 2016. Here's that commit for documentation purposes: 860b0fe

In that commit writeEventsBeforeWait and waitedEvents were also introduced together with write_in_use. If we don't need write_in_use then it could be need to remove these variables too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sadly, I don't think we have a PR for that commit. One possibility is that the in use tracking worked differently back then and that's why write_in_use and writeEventsBeforeWait, etc. were needed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 343545.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18622 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18622 passed.

@ziga-lunarg ziga-lunarg merged commit 5cceb78 into KhronosGroup:main Jan 13, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation of VUID 09543 does not handle all cases
4 participants