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

Move mixing out of the AudioStreamPlayer* nodes #51296

Merged
merged 3 commits into from
Aug 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
375 changes: 375 additions & 0 deletions core/templates/safe_list.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,375 @@
/*************************************************************************/
/* safe_list.h */
/*************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/*************************************************************************/
/* Copyright (c) 2007-2021 Juan Linietsky, Ariel Manzur. */
/* Copyright (c) 2014-2021 Godot Engine contributors (cf. AUTHORS.md). */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/*************************************************************************/

#ifndef SAFE_LIST_H
#define SAFE_LIST_H

#include "core/os/memory.h"
#include "core/typedefs.h"
#include <functional>

#if !defined(NO_THREADS)

#include <atomic>
#include <type_traits>

// Design goals for these classes:
// - Accessing this list with an iterator will never result in a use-after free,
// even if the element being accessed has been logically removed from the list on
// another thread.
// - Logical deletion from the list will not result in deallocation at that time,
// instead the node will be deallocated at a later time when it is safe to do so.
// - No blocking synchronization primitives will be used.
Comment on lines +43 to +49
Copy link
Member

Choose a reason for hiding this comment

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

CC @RandomShaper, you might be interested to review this class.

Copy link
Member

Choose a reason for hiding this comment

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

I almost skipped this. I don't think I'll be able to check it today, so please let me check it tomorrow.


// This is used in very specific areas of the engine where it's critical that these guarantees are held.

template <class T, class A = DefaultAllocator>
class SafeList {
struct SafeListNode {
std::atomic<SafeListNode *> next = nullptr;

// If the node is logically deleted, this pointer will typically point
// to the previous list item in time that was also logically deleted.
std::atomic<SafeListNode *> graveyard_next = nullptr;

std::function<void(T)> deletion_fn = [](T t) { return; };

T val;
};

static_assert(std::atomic<T>::is_always_lock_free);

std::atomic<SafeListNode *> head = nullptr;
std::atomic<SafeListNode *> graveyard_head = nullptr;

std::atomic_uint active_iterator_count = 0;

public:
class Iterator {
friend class SafeList;

SafeListNode *cursor;
SafeList *list;

Iterator(SafeListNode *p_cursor, SafeList *p_list) :
cursor(p_cursor), list(p_list) {
list->active_iterator_count++;
}

public:
Iterator(const Iterator &p_other) :
cursor(p_other.cursor), list(p_other.list) {
list->active_iterator_count++;
}

~Iterator() {
list->active_iterator_count--;
}

public:
T &operator*() {
return cursor->val;
}

Iterator &operator++() {
cursor = cursor->next;
return *this;
}

// These two operators are mostly useful for comparisons to nullptr.
bool operator==(const void *p_other) const {
return cursor == p_other;
}

bool operator!=(const void *p_other) const {
return cursor != p_other;
}

// These two allow easy range-based for loops.
bool operator==(const Iterator &p_other) const {
return cursor == p_other.cursor;
}

bool operator!=(const Iterator &p_other) const {
return cursor != p_other.cursor;
}
};

public:
// Calling this will cause an allocation.
void insert(T p_value) {
SafeListNode *new_node = memnew_allocator(SafeListNode, A);
new_node->val = p_value;
SafeListNode *expected_head = nullptr;
do {
expected_head = head.load();
new_node->next.store(expected_head);
} while (!head.compare_exchange_strong(/* expected= */ expected_head, /* new= */ new_node));
}

Iterator find(T p_value) {
for (Iterator it = begin(); it != end(); ++it) {
if (*it == p_value) {
return it;
}
}
return end();
}

void erase(T p_value, std::function<void(T)> p_deletion_fn) {
Iterator tmp = find(p_value);
erase(tmp, p_deletion_fn);
}

void erase(T p_value) {
Iterator tmp = find(p_value);
erase(tmp, [](T t) { return; });
}

void erase(Iterator &p_iterator, std::function<void(T)> p_deletion_fn) {
p_iterator.cursor->deletion_fn = p_deletion_fn;
erase(p_iterator);
}

void erase(Iterator &p_iterator) {
if (find(p_iterator.cursor->val) == nullptr) {
// Not in the list, nothing to do.
return;
}
// First, remove the node from the list.
while (true) {
Iterator prev = begin();
SafeListNode *expected_head = prev.cursor;
for (; prev != end(); ++prev) {
if (prev.cursor && prev.cursor->next == p_iterator.cursor) {
break;
}
}
if (prev != end()) {
// There exists a node before this.
prev.cursor->next.store(p_iterator.cursor->next.load());
// Done.
break;
} else {
if (head.compare_exchange_strong(/* expected= */ expected_head, /* new= */ p_iterator.cursor->next.load())) {
// Successfully reassigned the head pointer before another thread changed it to something else.
break;
}
// Fall through upon failure, try again.
}
}
// Then queue it for deletion by putting it in the node graveyard.
// Don't touch `next` because an iterator might still be pointing at this node.
SafeListNode *expected_head = nullptr;
do {
expected_head = graveyard_head.load();
p_iterator.cursor->graveyard_next.store(expected_head);
} while (!graveyard_head.compare_exchange_strong(/* expected= */ expected_head, /* new= */ p_iterator.cursor));
}

Iterator begin() {
return Iterator(head.load(), this);
}

Iterator end() {
return Iterator(nullptr, this);
}

// Calling this will cause zero to many deallocations.
void maybe_cleanup() {
SafeListNode *cursor = nullptr;
SafeListNode *new_graveyard_head = nullptr;
do {
// The access order here is theoretically important.
cursor = graveyard_head.load();
if (active_iterator_count.load() != 0) {
// It's not safe to clean up with an active iterator, because that iterator
// could be pointing to an element that we want to delete.
return;
}
// Any iterator created after this point will never point to a deleted node.
// Swap it out with the current graveyard head.
} while (!graveyard_head.compare_exchange_strong(/* expected= */ cursor, /* new= */ new_graveyard_head));
// Our graveyard list is now unreachable by any active iterators,
// detached from the main graveyard head and ready for deletion.
while (cursor) {
SafeListNode *tmp = cursor;
cursor = cursor->graveyard_next;
tmp->deletion_fn(tmp->val);
memdelete_allocator<SafeListNode, A>(tmp);
}
}
};

#else // NO_THREADS

// Effectively the same structure without the atomics. It's probably possible to simplify it but the semantics shouldn't differ greatly.
template <class T, class A = DefaultAllocator>
class SafeList {
struct SafeListNode {
SafeListNode *next = nullptr;

// If the node is logically deleted, this pointer will typically point to the previous list item in time that was also logically deleted.
SafeListNode *graveyard_next = nullptr;

std::function<void(T)> deletion_fn = [](T t) { return; };

T val;
};

SafeListNode *head = nullptr;
SafeListNode *graveyard_head = nullptr;

unsigned int active_iterator_count = 0;

public:
class Iterator {
friend class SafeList;

SafeListNode *cursor;
SafeList *list;

public:
Iterator(SafeListNode *p_cursor, SafeList *p_list) :
cursor(p_cursor), list(p_list) {
list->active_iterator_count++;
}

~Iterator() {
list->active_iterator_count--;
}

T &operator*() {
return cursor->val;
}

Iterator &operator++() {
cursor = cursor->next;
return *this;
}

// These two operators are mostly useful for comparisons to nullptr.
bool operator==(const void *p_other) const {
return cursor == p_other;
}

bool operator!=(const void *p_other) const {
return cursor != p_other;
}

// These two allow easy range-based for loops.
bool operator==(const Iterator &p_other) const {
return cursor == p_other.cursor;
}

bool operator!=(const Iterator &p_other) const {
return cursor != p_other.cursor;
}
};

public:
// Calling this will cause an allocation.
void insert(T p_value) {
SafeListNode *new_node = memnew_allocator(SafeListNode, A);
new_node->val = p_value;
new_node->next = head;
head = new_node;
}

Iterator find(T p_value) {
for (Iterator it = begin(); it != end(); ++it) {
if (*it == p_value) {
return it;
}
}
return end();
}

void erase(T p_value, std::function<void(T)> p_deletion_fn) {
erase(find(p_value), p_deletion_fn);
}

void erase(T p_value) {
erase(find(p_value), [](T t) { return; });
}

void erase(Iterator p_iterator, std::function<void(T)> p_deletion_fn) {
p_iterator.cursor->deletion_fn = p_deletion_fn;
erase(p_iterator);
}

void erase(Iterator p_iterator) {
Iterator prev = begin();
for (; prev != end(); ++prev) {
if (prev.cursor && prev.cursor->next == p_iterator.cursor) {
break;
}
}
if (prev == end()) {
// Not in the list, nothing to do.
return;
}
// First, remove the node from the list.
prev.cursor->next = p_iterator.cursor->next;

// Then queue it for deletion by putting it in the node graveyard. Don't touch `next` because an iterator might still be pointing at this node.
p_iterator.cursor->graveyard_next = graveyard_head;
graveyard_head = p_iterator.cursor;
}

Iterator begin() {
return Iterator(head, this);
}

Iterator end() {
return Iterator(nullptr, this);
}

// Calling this will cause zero to many deallocations.
void maybe_cleanup() {
SafeListNode *cursor = graveyard_head;
if (active_iterator_count != 0) {
// It's not safe to clean up with an active iterator, because that iterator could be pointing to an element that we want to delete.
return;
}
graveyard_head = nullptr;
// Our graveyard list is now unreachable by any active iterators, detached from the main graveyard head and ready for deletion.
while (cursor) {
SafeListNode *tmp = cursor;
cursor = cursor->next;
tmp->deletion_fn(tmp->val);
memdelete_allocator<SafeListNode, A>(tmp);
}
}
};

#endif

#endif // SAFE_LIST_H
2 changes: 1 addition & 1 deletion doc/classes/AudioStreamPlayback.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
</description>
</method>
<method name="_mix" qualifiers="virtual">
<return type="void" />
<return type="int" />
<argument index="0" name="buffer" type="AudioFrame*" />
<argument index="1" name="rate_scale" type="float" />
<argument index="2" name="frames" type="int" />
Expand Down
Loading