summaryrefslogtreecommitdiff
path: root/src/core/hle
diff options
context:
space:
mode:
authorZephyron <zephyron@citron-emu.org>2025-01-06 20:04:35 +1000
committerZephyron <zephyron@citron-emu.org>2025-01-06 20:04:35 +1000
commit66b6d5b2da4b65d11fe54c29c3babb8fcd98bfde (patch)
treec94db36e4279f085c54740b73d6e61b3a1d5a8ee /src/core/hle
parent83393a6c6bedc9c00378c73721ec17c7a4fb2b71 (diff)
core: Improve device memory and buffer queue handling
This commit makes several improvements to device memory management and buffer queue handling: DeviceMemoryManager: - Add null pointer and size checks for ReadBlock - Fill unmapped memory with a known pattern (0xCC) instead of zeros - Add better error logging for invalid memory accesses - Add null pointer check for source pointer in memcpy BufferQueueProducer: - Improve error handling in WaitForFreeSlotThenRelock - Add proper abandoned state checking during wait conditions - Clean up and simplify buffer scanning logic - Improve logging messages with more descriptive information - Remove redundant buffer count validation - Fix potential infinite loop condition during wait These changes improve stability and error handling while making the code more maintainable and debuggable. The use of a known pattern for unmapped memory helps identify uninitialized memory access issues.
Diffstat (limited to 'src/core/hle')
-rw-r--r--src/core/hle/service/nvnflinger/buffer_queue_producer.cpp47
1 files changed, 15 insertions, 32 deletions
diff --git a/src/core/hle/service/nvnflinger/buffer_queue_producer.cpp b/src/core/hle/service/nvnflinger/buffer_queue_producer.cpp
index 1430a70e8..58f07f60b 100644
--- a/src/core/hle/service/nvnflinger/buffer_queue_producer.cpp
+++ b/src/core/hle/service/nvnflinger/buffer_queue_producer.cpp
@@ -117,12 +117,14 @@ Status BufferQueueProducer::SetBufferCount(s32 buffer_count) {
}
Status BufferQueueProducer::WaitForFreeSlotThenRelock(bool async, s32* found, Status* return_flags,
- std::unique_lock<std::mutex>& lk) const {
+ std::unique_lock<std::mutex>& lk) const {
bool try_again = true;
while (try_again) {
+ // Check if queue is abandoned before proceeding
if (core->is_abandoned) {
LOG_ERROR(Service_Nvnflinger, "BufferQueue has been abandoned");
+ *found = BufferQueueCore::INVALID_BUFFER_SLOT;
return Status::NoInit;
}
@@ -136,7 +138,6 @@ Status BufferQueueProducer::WaitForFreeSlotThenRelock(bool async, s32* found, St
// Free up any buffers that are in slots beyond the max buffer count
for (s32 s = max_buffer_count; s < BufferQueueDefs::NUM_BUFFER_SLOTS; ++s) {
- ASSERT(slots[s].buffer_state == BufferState::Free);
if (slots[s].graphic_buffer != nullptr && slots[s].buffer_state == BufferState::Free &&
!slots[s].is_preallocated) {
core->FreeBufferLocked(s);
@@ -148,6 +149,8 @@ Status BufferQueueProducer::WaitForFreeSlotThenRelock(bool async, s32* found, St
*found = BufferQueueCore::INVALID_BUFFER_SLOT;
s32 dequeued_count{};
s32 acquired_count{};
+
+ // Scan for available buffers
for (s32 s{}; s < max_buffer_count; ++s) {
switch (slots[s].buffer_state) {
case BufferState::Dequeued:
@@ -157,8 +160,6 @@ Status BufferQueueProducer::WaitForFreeSlotThenRelock(bool async, s32* found, St
++acquired_count;
break;
case BufferState::Free:
- // We return the oldest of the free buffers to avoid stalling the producer if
- // possible, since the consumer may still have pending reads of in-flight buffers
if (*found == BufferQueueCore::INVALID_BUFFER_SLOT ||
slots[s].frame_number < slots[*found].frame_number) {
*found = s;
@@ -169,50 +170,32 @@ Status BufferQueueProducer::WaitForFreeSlotThenRelock(bool async, s32* found, St
}
}
- // Producers are not allowed to dequeue more than one buffer if they did not set a buffer
- // count
+ // Check for buffer count override issues
if (!core->override_max_buffer_count && dequeued_count) {
- LOG_ERROR(Service_Nvnflinger,
- "can't dequeue multiple buffers without setting the buffer count");
+ LOG_ERROR(Service_Nvnflinger, "Can't dequeue multiple buffers without setting buffer count");
return Status::InvalidOperation;
}
- // See whether a buffer has been queued since the last SetBufferCount so we know whether to
- // perform the min undequeued buffers check below
- if (core->buffer_has_been_queued) {
- // Make sure the producer is not trying to dequeue more buffers than allowed
- const s32 new_undequeued_count = max_buffer_count - (dequeued_count + 1);
- const s32 min_undequeued_count = core->GetMinUndequeuedBufferCountLocked(async);
- if (new_undequeued_count < min_undequeued_count) {
- LOG_ERROR(Service_Nvnflinger,
- "min undequeued buffer count({}) exceeded (dequeued={} undequeued={})",
- min_undequeued_count, dequeued_count, new_undequeued_count);
- return Status::InvalidOperation;
- }
- }
-
- // If we disconnect and reconnect quickly, we can be in a state where our slots are empty
- // but we have many buffers in the queue. This can cause us to run out of memory if we
- // outrun the consumer. Wait here if it looks like we have too many buffers queued up.
+ // Handle queue size limits
const bool too_many_buffers = core->queue.size() > static_cast<size_t>(max_buffer_count);
if (too_many_buffers) {
- LOG_ERROR(Service_Nvnflinger, "queue size is {}, waiting", core->queue.size());
+ LOG_WARNING(Service_Nvnflinger, "Queue size {} exceeds max buffer count {}, waiting",
+ core->queue.size(), max_buffer_count);
}
- // If no buffer is found, or if the queue has too many buffers outstanding, wait for a
- // buffer to be acquired or released, or for the max buffer count to change.
try_again = (*found == BufferQueueCore::INVALID_BUFFER_SLOT) || too_many_buffers;
if (try_again) {
- // Return an error if we're in non-blocking mode (producer and consumer are controlled
- // by the application).
if (core->dequeue_buffer_cannot_block &&
(acquired_count <= core->max_acquired_buffer_count)) {
return Status::WouldBlock;
}
if (!core->WaitForDequeueCondition(lk)) {
- // We are no longer running
- return Status::NoError;
+ if (core->is_abandoned) {
+ LOG_ERROR(Service_Nvnflinger, "BufferQueue was abandoned while waiting");
+ return Status::NoInit;
+ }
+ continue;
}
}
}