From Jérôme Duval <jerome.duval@xxxxxxxxx>:
Jérôme Duval has uploaded this change for review. (
https://review.haiku-os.org/c/haiku/+/2245 ;)
Change subject: Media Kit: BBufferCache: if not reclaimed, only mark the buffer
for deletion
......................................................................
Media Kit: BBufferCache: if not reclaimed, only mark the buffer for deletion
hrev53379 clears the buffer cache for disconnected clients, and also delete
buffers.
This is too early (see #15263, media_addon_server crash), and should only happen
after the buffer is recycled. This can be resolved by abusing the fFlags field
of
BBuffer to mark the buffer for deletion, and mark the buffer to be reclaimed.
Some BBuffers don't reside in the SharedBufferList, so we have to mark them as
to
be reclaimed. For those in the SharedBufferList, call a new RemoveBuffer(),
which
can check whether the buffer is still to be reclaimed. For reclaimed BBuffers,
delete them right away, others can be marked for deletion.
fixes #15606 #15263, possibly #15433
---
M headers/private/media/MediaMisc.h
M headers/private/media/SharedBufferList.h
M src/kits/media/Buffer.cpp
M src/kits/media/BufferCache.cpp
M src/kits/media/SharedBufferList.cpp
5 files changed, 70 insertions(+), 4 deletions(-)
git pull ssh://git.haiku-os.org:22/haiku refs/changes/45/2245/1
diff --git a/headers/private/media/MediaMisc.h
b/headers/private/media/MediaMisc.h
index b22944c..b422b03 100644
--- a/headers/private/media/MediaMisc.h
+++ b/headers/private/media/MediaMisc.h
@@ -44,6 +44,10 @@
#define MEDIA_SERVER_PORT_NAME "__media_server_port"
#define MEDIA_ADDON_SERVER_PORT_NAME "__media_addon_server_port"
+#define BUFFER_TO_RECLAIM 0x20000000
+#define BUFFER_MARKED_FOR_DELETION 0x40000000
+
+
extern const char *B_MEDIA_ADDON_SERVER_SIGNATURE;
namespace BPrivate { namespace media {
diff --git a/headers/private/media/SharedBufferList.h
b/headers/private/media/SharedBufferList.h
index 000165d..9006491 100644
--- a/headers/private/media/SharedBufferList.h
+++ b/headers/private/media/SharedBufferList.h
@@ -30,6 +30,7 @@
status_t
AddBuffer(sem_id groupReclaimSem,
const buffer_clone_info& info,
BBuffer** buffer);
+ status_t
RemoveBuffer(BBuffer* buffer);
// Call AddBuffer and CheckID locked
status_t
AddBuffer(sem_id groupReclaimSem,
diff --git a/src/kits/media/Buffer.cpp b/src/kits/media/Buffer.cpp
index 6d1988c..1d17a9c 100644
--- a/src/kits/media/Buffer.cpp
+++ b/src/kits/media/Buffer.cpp
@@ -38,6 +38,7 @@
#include <MediaDefs.h>
#include "MediaDebug.h"
+#include "MediaMisc.h"
#include "DataExchange.h"
#include "SharedBufferList.h"
@@ -114,7 +115,11 @@
CALLED();
if (fBufferList == NULL)
return;
- fBufferList->RecycleBuffer(this);
+ fFlags &= ~BUFFER_TO_RECLAIM;
+ if ((fFlags & BUFFER_MARKED_FOR_DELETION) != 0)
+ delete this;
+ else
+ fBufferList->RecycleBuffer(this);
}
diff --git a/src/kits/media/BufferCache.cpp b/src/kits/media/BufferCache.cpp
index cbcc6ac..9ec71d6 100644
--- a/src/kits/media/BufferCache.cpp
+++ b/src/kits/media/BufferCache.cpp
@@ -14,6 +14,8 @@
#include <Buffer.h>
#include "MediaDebug.h"
+#include "MediaMisc.h"
+#include "SharedBufferList.h"
namespace BPrivate {
@@ -41,8 +43,10 @@
return NULL;
buffer_cache_entry* existing;
- if (fMap.Get(id, existing))
+ if (fMap.Get(id, existing)) {
+ existing->buffer->fFlags |= BUFFER_TO_RECLAIM;
return existing->buffer;
+ }
buffer_clone_info info;
info.buffer = id;
@@ -65,6 +69,7 @@
return NULL;
}
+ buffer->fFlags |= BUFFER_TO_RECLAIM;
return buffer;
}
@@ -76,8 +81,17 @@
while (iterator.HasNext()) {
BufferMap::Entry entry = iterator.Next();
if (entry.value.port == port) {
- // Delete the buffer
- delete entry.value.buffer;
+ BBuffer* buffer = entry.value.buffer;
+ bool isReclaimed = (buffer->fFlags & BUFFER_TO_RECLAIM)
== 0;
+ if (isReclaimed && buffer->fBufferList != NULL)
+ isReclaimed =
buffer->fBufferList->RemoveBuffer(buffer) == B_OK;
+
+ if (isReclaimed)
+ delete buffer;
+ else {
+ // mark the buffer for deletion
+ buffer->fFlags |= BUFFER_MARKED_FOR_DELETION;
+ }
// Then remove it from the map
fMap.Remove(iterator);
}
diff --git a/src/kits/media/SharedBufferList.cpp
b/src/kits/media/SharedBufferList.cpp
index fed865b..5673874 100644
--- a/src/kits/media/SharedBufferList.cpp
+++ b/src/kits/media/SharedBufferList.cpp
@@ -378,6 +378,48 @@
}
+status_t
+SharedBufferList::RemoveBuffer(BBuffer* buffer)
+{
+ CALLED();
+
+ media_buffer_id id = buffer->ID();
+
+ if (Lock() != B_OK)
+ return B_ERROR;
+
+ int32 notRemovedCount = 0;
+
+ for (int32 i = 0; i < fCount; i++) {
+ // find the buffer id, and remove it in all groups it belongs to
+ if (fInfos[i].id == id) {
+ if (!fInfos[i].reclaimed) {
+ notRemovedCount++;
+ ERROR("SharedBufferList::RequestBuffer, BBuffer
%p, id = %"
+ B_PRId32 " not reclaimed\n", buffer,
id);
+ DEBUG_ONLY(debugger("buffer not reclaimed"));
+ continue;
+ }
+ fInfos[i].buffer = NULL;
+ fInfos[i].id = -1;
+ fInfos[i].reclaim_sem = -1;
+ }
+ }
+
+ if (Unlock() != B_OK)
+ return B_ERROR;
+
+ if (notRemovedCount != 0) {
+ ERROR("SharedBufferList::RemoveBuffer, BBuffer %p, id = %"
B_PRId32
+ " not reclaimed\n", buffer, id);
+ return B_ERROR;
+ }
+
+ return B_OK;
+}
+
+
+
/*! Returns exactly \a bufferCount buffers from the group specified via its
\a groupReclaimSem if successful.
*/
--
To view, visit https://review.haiku-os.org/c/haiku/+/2245
To unsubscribe, or for help writing mail filters, visit
https://review.haiku-os.org/settings
Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: I66e94138e7e10a40d4c48e2ac042f816c79f5aab
Gerrit-Change-Number: 2245
Gerrit-PatchSet: 1
Gerrit-Owner: Jérôme Duval <jerome.duval@xxxxxxxxx>
Gerrit-MessageType: newchange
