1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-27 04:27:45 +00:00

LibCore: Make CObject reference-counted

Okay, I've spent a whole day on this now, and it finally kinda works!
With this patch, CObject and all of its derived classes are reference
counted instead of tree-owned.

The previous, Qt-like model was nice and familiar, but ultimately also
outdated and difficult to reason about.

CObject-derived types should now be stored in RefPtr/NonnullRefPtr and
each class can be constructed using the forwarding construct() helper:

    auto widget = GWidget::construct(parent_widget);

Note that construct() simply forwards all arguments to an existing
constructor. It is inserted into each class by the C_OBJECT macro,
see CObject.h to understand how that works.

CObject::delete_later() disappears in this patch, as there is no longer
a single logical owner of a CObject.
This commit is contained in:
Andreas Kling 2019-09-22 00:17:53 +02:00
parent 0c72e0c09f
commit bc319d9e88
45 changed files with 174 additions and 233 deletions

View file

@ -15,7 +15,6 @@ public:
Timer,
NotifierRead,
NotifierWrite,
DeferredDestroy,
DeferredInvoke,
ChildAdded,
ChildRemoved,

View file

@ -43,7 +43,7 @@ public:
int nread = m_socket->read((u8*)&length, sizeof(length));
if (nread == 0) {
dbg() << "RPC client disconnected";
delete_later();
shutdown();
return;
}
ASSERT(nread == sizeof(length));
@ -52,7 +52,7 @@ public:
auto request_json = JsonValue::from_string(request);
if (!request_json.is_object()) {
dbg() << "RPC client sent invalid request";
delete_later();
shutdown();
return;
}
@ -111,11 +111,17 @@ public:
}
if (type == "Disconnect") {
delete_later();
shutdown();
return;
}
}
void shutdown()
{
// FIXME: This is quite a hackish way to clean ourselves up.
delete this;
}
private:
ObjectPtr<CLocalSocket> m_socket;
};
@ -148,7 +154,8 @@ CEventLoop::CEventLoop()
s_rpc_server->on_ready_to_accept = [&] {
auto client_socket = s_rpc_server->accept();
ASSERT(client_socket);
new RPCClient(move(client_socket));
// NOTE: RPCClient will delete itself in RPCClient::shutdown().
(void)RPCClient::construct(move(client_socket)).leak_ref();
};
}
@ -246,6 +253,7 @@ void CEventLoop::pump(WaitMode mode)
#endif
static_cast<CDeferredInvocationEvent&>(event).m_invokee(*receiver);
} else {
NonnullRefPtr<CObject> protector(*receiver);
receiver->dispatch_event(event);
}

View file

@ -23,13 +23,18 @@ CObject::CObject(CObject* parent, bool is_widget)
CObject::~CObject()
{
// NOTE: We move our children out to a stack vector to prevent other
// code from trying to iterate over them.
auto children = move(m_children);
// NOTE: We also unparent the children, so that they won't try to unparent
// themselves in their own destructors.
for (auto& child : children)
child.m_parent = nullptr;
all_objects().remove(*this);
stop_timer();
if (m_parent)
m_parent->remove_child(*this);
auto children_to_delete = move(m_children);
for (auto* child : children_to_delete)
delete child;
}
void CObject::event(CEvent& event)
@ -37,9 +42,6 @@ void CObject::event(CEvent& event)
switch (event.type()) {
case CEvent::Timer:
return timer_event(static_cast<CTimerEvent&>(event));
case CEvent::DeferredDestroy:
delete this;
break;
case CEvent::ChildAdded:
case CEvent::ChildRemoved:
return child_event(static_cast<CChildEvent&>(event));
@ -58,19 +60,24 @@ void CObject::add_child(CObject& object)
// FIXME: Should we support reparenting objects?
ASSERT(!object.parent() || object.parent() == this);
object.m_parent = this;
m_children.append(&object);
m_children.append(object);
event(*make<CChildEvent>(CEvent::ChildAdded, object));
}
void CObject::remove_child(CObject& object)
{
for (ssize_t i = 0; i < m_children.size(); ++i) {
if (m_children[i] == &object) {
for (int i = 0; i < m_children.size(); ++i) {
dbg() << i << "] " << m_children.at(i);
if (m_children.ptr_at(i).ptr() == &object) {
// NOTE: We protect the child so it survives the handling of ChildRemoved.
NonnullRefPtr<CObject> protector = object;
object.m_parent = nullptr;
m_children.remove(i);
event(*make<CChildEvent>(CEvent::ChildRemoved, object));
return;
}
}
ASSERT_NOT_REACHED();
}
void CObject::timer_event(CTimerEvent&)
@ -104,11 +111,6 @@ void CObject::stop_timer()
m_timer_id = 0;
}
void CObject::delete_later()
{
CEventLoop::current().post_event(*this, make<CEvent>(CEvent::DeferredDestroy));
}
void CObject::dump_tree(int indent)
{
for (int i = 0; i < indent; ++i) {

View file

@ -2,6 +2,8 @@
#include <AK/Function.h>
#include <AK/IntrusiveList.h>
#include <AK/Noncopyable.h>
#include <AK/NonnullRefPtrVector.h>
#include <AK/StdLibExtras.h>
#include <AK/String.h>
#include <AK/Vector.h>
@ -21,13 +23,18 @@ class CTimerEvent;
public: \
virtual const char* class_name() const override { return #klass; } \
template<class... Args> \
static inline ObjectPtr<klass> construct(Args&&... args) \
static inline NonnullRefPtr<klass> construct(Args&&... args) \
{ \
return ObjectPtr<klass>(new klass(forward<Args>(args)...)); \
return adopt(*new klass(forward<Args>(args)...)); \
}
class CObject : public Weakable<CObject> {
class CObject
: public RefCounted<CObject>
, public Weakable<CObject> {
// NOTE: No C_OBJECT macro for CObject itself.
AK_MAKE_NONCOPYABLE(CObject)
AK_MAKE_NONMOVABLE(CObject)
public:
IntrusiveListNode m_all_objects_list_node;
@ -39,14 +46,14 @@ public:
const String& name() const { return m_name; }
void set_name(const StringView& name) { m_name = name; }
Vector<CObject*>& children() { return m_children; }
const Vector<CObject*>& children() const { return m_children; }
NonnullRefPtrVector<CObject>& children() { return m_children; }
const NonnullRefPtrVector<CObject>& children() const { return m_children; }
template<typename Callback>
void for_each_child(Callback callback)
{
for (auto* child : m_children) {
if (callback(*child) == IterationDecision::Break)
for (auto& child : m_children) {
if (callback(child) == IterationDecision::Break)
return;
}
}
@ -66,8 +73,6 @@ public:
void add_child(CObject&);
void remove_child(CObject&);
void delete_later();
void dump_tree(int indent = 0);
void deferred_invoke(Function<void(CObject&)>);
@ -95,7 +100,7 @@ private:
String m_name;
int m_timer_id { 0 };
bool m_widget { false };
Vector<CObject*> m_children;
NonnullRefPtrVector<CObject> m_children;
};
template<typename T>

View file

@ -49,22 +49,22 @@ namespace Server {
};
template<typename T, class... Args>
T* new_connection_for_client(Args&&... args)
NonnullRefPtr<T> new_connection_for_client(Args&&... args)
{
auto conn = new T(AK::forward<Args>(args)...) /* arghs */;
auto conn = T::construct(forward<Args>(args)...);
conn->send_greeting();
return conn;
}
template<typename T, class... Args>
T* new_connection_ng_for_client(Args&&... args)
NonnullRefPtr<T> new_connection_ng_for_client(Args&&... args)
{
return new T(AK::forward<Args>(args)...) /* arghs */;
return T::construct(forward<Args>(args)...) /* arghs */;
}
template<typename ServerMessage, typename ClientMessage>
class Connection : public CObject {
public:
protected:
Connection(CLocalSocket& socket, int client_id)
: m_socket(socket)
, m_client_id(client_id)
@ -76,6 +76,7 @@ namespace Server {
#endif
}
public:
~Connection()
{
#if defined(CIPC_DEBUG)
@ -108,14 +109,12 @@ namespace Server {
switch (errno) {
case EPIPE:
dbgprintf("Connection::post_message: Disconnected from peer.\n");
delete_later();
shutdown();
return;
break;
case EAGAIN:
dbgprintf("Connection::post_message: Client buffer overflowed.\n");
did_misbehave();
return;
break;
default:
perror("Connection::post_message writev");
ASSERT_NOT_REACHED();
@ -134,7 +133,6 @@ namespace Server {
ssize_t nread = recv(m_socket->fd(), &message, sizeof(ClientMessage), MSG_DONTWAIT);
if (nread == 0 || (nread == -1 && errno == EAGAIN)) {
if (!messages_received) {
// TODO: is delete_later() sufficient?
CEventLoop::current().post_event(*this, make<DisconnectedEvent>(client_id()));
}
break;
@ -171,8 +169,13 @@ namespace Server {
void did_misbehave()
{
dbgprintf("Connection{%p} (id=%d, pid=%d) misbehaved, disconnecting.\n", this, client_id(), m_client_pid);
shutdown();
}
void shutdown()
{
m_socket->close();
delete_later();
die();
}
int client_id() const { return m_client_id; }
@ -182,13 +185,15 @@ namespace Server {
// ### having this public is sad
virtual void send_greeting() = 0;
virtual void die() = 0;
protected:
void event(CEvent& event)
{
if (event.type() == Event::Disconnected) {
int client_id = static_cast<const DisconnectedEvent&>(event).client_id();
dbgprintf("Connection: Client disconnected: %d\n", client_id);
delete this;
die();
return;
}
@ -228,13 +233,12 @@ namespace Server {
switch (errno) {
case EPIPE:
dbg() << "Connection::post_message: Disconnected from peer";
delete_later();
shutdown();
return;
case EAGAIN:
dbg() << "Connection::post_message: Client buffer overflowed.";
did_misbehave();
return;
break;
default:
perror("Connection::post_message write");
ASSERT_NOT_REACHED();
@ -252,7 +256,6 @@ namespace Server {
ssize_t nread = recv(m_socket->fd(), buffer, sizeof(buffer), MSG_DONTWAIT);
if (nread == 0 || (nread == -1 && errno == EAGAIN)) {
if (!messages_received) {
// TODO: is delete_later() sufficient?
CEventLoop::current().post_event(*this, make<DisconnectedEvent>(client_id()));
}
break;
@ -278,21 +281,28 @@ namespace Server {
void did_misbehave()
{
dbg() << "Connection{" << this << "} (id=" << m_client_id << ", pid=" << m_client_pid << ") misbehaved, disconnecting.";
shutdown();
}
void shutdown()
{
m_socket->close();
delete_later();
die();
}
int client_id() const { return m_client_id; }
pid_t client_pid() const { return m_client_pid; }
void set_client_pid(pid_t pid) { m_client_pid = pid; }
virtual void die() = 0;
protected:
void event(CEvent& event) override
{
if (event.type() == Event::Disconnected) {
int client_id = static_cast<const DisconnectedEvent&>(event).client_id();
dbgprintf("Connection: Client disconnected: %d\n", client_id);
delete this;
die();
return;
}

View file

@ -1,99 +1,3 @@
#pragma once
#include <AK/StdLibExtras.h>
// This is a stopgap pointer. It's not meant to stick around forever.
template<typename T>
class ObjectPtr {
public:
ObjectPtr() {}
ObjectPtr(T* ptr)
: m_ptr(ptr)
{
}
ObjectPtr(T& ptr)
: m_ptr(&ptr)
{
}
~ObjectPtr()
{
clear();
}
void clear()
{
if (m_ptr && !m_ptr->parent())
delete m_ptr;
m_ptr = nullptr;
}
ObjectPtr& operator=(std::nullptr_t)
{
clear();
return *this;
}
template<typename U>
ObjectPtr(U* ptr)
: m_ptr(static_cast<T*>(ptr))
{
}
ObjectPtr(const ObjectPtr& other)
: m_ptr(other.m_ptr)
{
}
template<typename U>
ObjectPtr(const ObjectPtr<U>& other)
: m_ptr(static_cast<T*>(const_cast<ObjectPtr<U>&>(other).ptr()))
{
}
ObjectPtr(ObjectPtr&& other)
{
m_ptr = other.leak_ptr();
}
template<typename U>
ObjectPtr(const ObjectPtr<U>&& other)
{
m_ptr = static_cast<T*>(const_cast<ObjectPtr<U>&>(other).leak_ptr());
}
ObjectPtr& operator=(const ObjectPtr& other)
{
if (this != &other) {
clear();
m_ptr = other.m_ptr;
}
return *this;
}
ObjectPtr& operator=(ObjectPtr&& other)
{
if (this != &other) {
clear();
m_ptr = exchange(other.m_ptr, nullptr);
}
return *this;
}
T* operator->() { return m_ptr; }
const T* operator->() const { return m_ptr; }
operator T*() { return m_ptr; }
operator const T*() const { return m_ptr; }
T& operator*() { return *m_ptr; }
const T& operator*() const { return *m_ptr; }
T* ptr() const { return m_ptr; }
T* leak_ptr() { return exchange(m_ptr, nullptr); }
operator bool() const { return !!m_ptr; }
private:
T* m_ptr { nullptr };
};
#define ObjectPtr RefPtr