From 45009643949407fcf53036a7e142203ae763cf78 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 30 Aug 2013 10:16:28 -0700 Subject: [PATCH] Use Refcounted to manage to NativeEntry (bug 5852 part 4, r=ds). --HG-- extra : rebase_source : 2e08816db6819c9d9957a0e0ade9cd1aa420fd54 --- core/logic/Native.h | 98 ++++++++++++++++++++++++++++++ core/logic/NativeInvoker.cpp | 4 +- core/logic/NativeOwner.cpp | 28 +++------ core/logic/NativeOwner.h | 15 ++--- core/logic/PluginSys.cpp | 50 +++++----------- core/logic/ShareSys.cpp | 112 +++++++++++++---------------------- core/logic/ShareSys.h | 79 ++++-------------------- public/amtl/am-refcounting.h | 15 ++++- public/sm_namehashset.h | 34 +++++++++++ 9 files changed, 227 insertions(+), 208 deletions(-) create mode 100644 core/logic/Native.h diff --git a/core/logic/Native.h b/core/logic/Native.h new file mode 100644 index 00000000..b11304ab --- /dev/null +++ b/core/logic/Native.h @@ -0,0 +1,98 @@ +/** + * vim: set ts=4 sw=4 tw=99 noet : + * ============================================================================= + * SourceMod + * Copyright (C) 2004-2008 AlliedModders LLC. All rights reserved. + * ============================================================================= + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License, version 3.0, as published by the + * Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + * + * As a special exception, AlliedModders LLC gives you permission to link the + * code of this program (as well as its derivative works) to "Half-Life 2," the + * "Source Engine," the "SourcePawn JIT," and any Game MODs that run on software + * by the Valve Corporation. You must obey the GNU General Public License in + * all respects for all other code used. Additionally, AlliedModders LLC grants + * this exception to all derivative works. AlliedModders LLC defines further + * exceptions, found in LICENSE.txt (as of this writing, version JULY-31-2007), + * or . + * + * Version: $Id$ + */ +#ifndef _include_sourcemod_native_h_ +#define _include_sourcemod_native_h_ + +#include +#include +#include +#include +#include +#include "common_logic.h" + +struct FakeNative +{ + FakeNative(const char *name, IPluginFunction *fun) + : name(name), + ctx(fun->GetParentContext()), + call(fun), + gate(NULL) + { + } + ~FakeNative(); + + ke::AString name; + IPluginContext *ctx; + IPluginFunction *call; + SPVM_NATIVE_FUNC gate; +}; + +struct Native : public ke::Refcounted +{ + Native(CNativeOwner *owner, const sp_nativeinfo_t *native) + : owner(owner), + native(native), + fake(NULL) + { + } + Native(CNativeOwner *owner, FakeNative *fake) + : owner(owner), + native(NULL), + fake(fake) + { + } + + CNativeOwner *owner; + const sp_nativeinfo_t *native; + ke::AutoPtr fake; + + SPVM_NATIVE_FUNC func() const + { + if (native) + return native->func; + return fake->gate; + } + const char *name() const + { + if (native) + return native->name; + return fake->name.chars(); + } + + static inline bool matches(const char *name, const ke::Ref &entry) + { + return strcmp(name, entry->name()) == 0; + } +}; + + +#endif // _include_sourcemod_native_h_ + diff --git a/core/logic/NativeInvoker.cpp b/core/logic/NativeInvoker.cpp index 9406c7b8..9698bd7a 100644 --- a/core/logic/NativeInvoker.cpp +++ b/core/logic/NativeInvoker.cpp @@ -69,11 +69,11 @@ INativeInvoker *NativeInterface::CreateInvoker() bool NativeInvoker::Start(IPluginContext *pContext, const char *name) { - NativeEntry *entry = g_ShareSys.FindNative(name); + ke::Ref entry = g_ShareSys.FindNative(name); if (!entry) return false; - if (!entry->owner || !entry->func()) + if (!entry->owner) return false; native_ = entry->func(); diff --git a/core/logic/NativeOwner.cpp b/core/logic/NativeOwner.cpp index 6bcce341..bb62f837 100644 --- a/core/logic/NativeOwner.cpp +++ b/core/logic/NativeOwner.cpp @@ -78,7 +78,7 @@ void CNativeOwner::PropagateMarkSerial(unsigned int serial) } } -void CNativeOwner::UnbindWeakRef(const WeakNative & ref) +void CNativeOwner::UnbindWeakRef(const WeakNative &ref) { sp_native_t *native; IPluginContext *pContext; @@ -86,25 +86,13 @@ void CNativeOwner::UnbindWeakRef(const WeakNative & ref) pContext = ref.pl->GetBaseContext(); if ((pContext->GetNativeByIndex(ref.idx, &native)) == SP_ERROR_NONE) { - /* If there is no reference, the native must be unbound */ - if (ref.entry == NULL) - { - native->status = SP_NATIVE_UNBOUND; - native->pfn = NULL; - } - /* If we've cached a reference, it's a core native we can - * rebind back to its original (this was a replacement). - */ - else - { - native->pfn = ref.entry->func(); - } + native->status = SP_NATIVE_UNBOUND; + native->pfn = NULL; } } void CNativeOwner::DropEverything() { - NativeEntry *pEntry; List::iterator iter; /* Unbind and remove all weak references to us */ @@ -121,13 +109,11 @@ void CNativeOwner::DropEverything() for (const sp_nativeinfo_t *native = natives; native->func && native->name; native++) g_ShareSys.ClearNativeFromCache(this, native->name); } + m_natives.clear(); - List::iterator ntv_iter = m_Natives.begin(); - while (ntv_iter != m_Natives.end()) - { - g_ShareSys.ClearNativeFromCache(this, (*ntv_iter)->name()); - ntv_iter = m_Natives.erase(ntv_iter); - } + for (size_t i = 0; i < m_fakes.length(); i++) + g_ShareSys.ClearNativeFromCache(this, m_fakes[i]->name()); + m_fakes.clear(); } void CNativeOwner::DropWeakRefsTo(CPlugin *pPlugin) diff --git a/core/logic/NativeOwner.h b/core/logic/NativeOwner.h index e3f9f872..43b79ee2 100644 --- a/core/logic/NativeOwner.h +++ b/core/logic/NativeOwner.h @@ -33,10 +33,12 @@ #include #include +#include #include #include "common_logic.h" +#include "Native.h" -struct NativeEntry; +struct Native; class CPlugin; using namespace SourceMod; @@ -44,20 +46,13 @@ using namespace SourceMod; struct WeakNative { WeakNative(IPlugin *plugin, uint32_t index) : - pl(plugin), idx(index), entry(NULL) - { - pl = plugin; - idx = index; - } - WeakNative(IPlugin *plugin, uint32_t index, NativeEntry *pEntry) : - pl(plugin), idx(index), entry(pEntry) + pl(plugin), idx(index) { pl = plugin; idx = index; } IPlugin *pl; uint32_t idx; - NativeEntry *entry; }; using namespace SourceHook; @@ -86,7 +81,7 @@ protected: unsigned int m_nMarkSerial; List m_WeakRefs; ke::Vector m_natives; - List m_Natives; + ke::Vector > m_fakes; }; extern CNativeOwner g_CoreNatives; diff --git a/core/logic/PluginSys.cpp b/core/logic/PluginSys.cpp index c9b865fb..0526cee3 100644 --- a/core/logic/PluginSys.cpp +++ b/core/logic/PluginSys.cpp @@ -596,9 +596,7 @@ IPhraseCollection *CPlugin::GetPhrases() void CPlugin::DependencyDropped(CPlugin *pOwner) { if (!m_pRuntime) - { return; - } List::iterator reqlib_iter; List::iterator lib_iter; @@ -607,30 +605,22 @@ void CPlugin::DependencyDropped(CPlugin *pOwner) for (reqlib_iter=m_RequiredLibs.begin(); reqlib_iter!=m_RequiredLibs.end(); reqlib_iter++) { if ((*reqlib_iter) == (*lib_iter)) - { m_LibraryMissing = true; - } } } - List::iterator iter; - NativeEntry *pNative; - sp_native_t *native; - uint32_t idx; unsigned int unbound = 0; - - for (iter = pOwner->m_Natives.begin(); - iter != pOwner->m_Natives.end(); - iter++) + for (size_t i = 0; i < pOwner->m_fakes.length(); i++) { - pNative = (*iter); - /* Find this native! */ - if (m_pRuntime->FindNativeByName(pNative->name(), &idx) != SP_ERROR_NONE) - { + ke::Ref entry(pOwner->m_fakes[i]); + + uint32_t idx; + if (m_pRuntime->FindNativeByName(entry->name(), &idx) != SP_ERROR_NONE) continue; - } - /* Unbind it */ + + sp_native_t *native; m_pRuntime->GetNativeByIndex(idx, &native); + native->pfn = NULL; native->status = SP_NATIVE_UNBOUND; unbound++; @@ -722,15 +712,11 @@ void CPlugin::DropEverything() bool CPlugin::AddFakeNative(IPluginFunction *pFunc, const char *name, SPVM_FAKENATIVE_FUNC func) { - NativeEntry *pEntry; - - if ((pEntry = g_ShareSys.AddFakeNative(pFunc, name, func)) == NULL) - { + ke::Ref entry = g_ShareSys.AddFakeNative(pFunc, name, func); + if (!entry) return false; - } - - m_Natives.push_back(pEntry); + m_fakes.append(entry); return true; } @@ -1383,15 +1369,14 @@ bool CPluginManager::RunSecondPass(CPlugin *pPlugin, char *error, size_t maxleng pPlugin->Call_OnPluginStart(); /* Now, if we have fake natives, go through all plugins that might need rebinding */ - if (pPlugin->GetStatus() <= Plugin_Paused && pPlugin->m_Natives.size()) + if (pPlugin->GetStatus() <= Plugin_Paused && pPlugin->m_fakes.length()) { List::iterator pl_iter; - CPlugin *pOther; for (pl_iter = m_plugins.begin(); pl_iter != m_plugins.end(); pl_iter++) { - pOther = (*pl_iter); + CPlugin *pOther = (*pl_iter); if ((pOther->GetStatus() == Plugin_Error && (pOther->m_FakeNativesMissing || pOther->m_LibraryMissing)) || pOther->m_FakeNativesMissing) @@ -1402,13 +1387,8 @@ bool CPluginManager::RunSecondPass(CPlugin *pPlugin, char *error, size_t maxleng || pOther->GetStatus() == Plugin_Paused) && pOther != pPlugin) { - List::iterator nv_iter; - for (nv_iter = pPlugin->m_Natives.begin(); - nv_iter != pPlugin->m_Natives.end(); - nv_iter++) - { - g_ShareSys.BindNativeToPlugin(pOther, (*nv_iter)); - } + for (size_t i = 0; i < pPlugin->m_fakes.length(); i++) + g_ShareSys.BindNativeToPlugin(pOther, pPlugin->m_fakes[i]); } } } diff --git a/core/logic/ShareSys.cpp b/core/logic/ShareSys.cpp index 768ab148..50f80f43 100644 --- a/core/logic/ShareSys.cpp +++ b/core/logic/ShareSys.cpp @@ -37,6 +37,8 @@ #include "HandleSys.h" #include +using namespace ke; + ShareSystem g_ShareSys; static unsigned int g_mark_serial = 0; @@ -259,12 +261,12 @@ void ShareSystem::OverrideNatives(IExtension *myself, const sp_nativeinfo_t *nat assert(false); } -NativeEntry *ShareSystem::FindNative(const char *name) +PassRef ShareSystem::FindNative(const char *name) { - NativeEntry *entry; - if (!m_NtvCache.retrieve(name, &entry)) + NativeCache::Result r = m_NtvCache.find(name); + if (!r.found()) return NULL; - return entry; + return *r; } void ShareSystem::BindNativesToPlugin(CPlugin *pPlugin, bool bCoreOnly) @@ -290,7 +292,7 @@ void ShareSystem::BindNativesToPlugin(CPlugin *pPlugin, bool bCoreOnly) continue; /* Otherwise, the native must be in our cache. */ - NativeEntry *pEntry = FindNative(native->name); + Ref pEntry = FindNative(native->name); if (!pEntry) continue; @@ -301,41 +303,32 @@ void ShareSystem::BindNativesToPlugin(CPlugin *pPlugin, bool bCoreOnly) } } -void ShareSystem::BindNativeToPlugin(CPlugin *pPlugin, NativeEntry *pEntry) +void ShareSystem::BindNativeToPlugin(CPlugin *pPlugin, const Ref &entry) { + if (!entry->owner) + return; + + IPluginContext *pContext = pPlugin->GetBaseContext(); + uint32_t i; + if (pContext->FindNativeByName(entry->name(), &i) != SP_ERROR_NONE) + return; + sp_native_t *native; - IPluginContext *pContext; - - pContext = pPlugin->GetBaseContext(); - - if (pContext->FindNativeByName(pEntry->name(), &i) != SP_ERROR_NONE) - { - return; - } if (pContext->GetNativeByIndex(i, &native) != SP_ERROR_NONE) - { return; - } if (native->status == SP_NATIVE_BOUND) - { return; - } - BindNativeToPlugin(pPlugin, native, i, pEntry); + BindNativeToPlugin(pPlugin, native, i, entry); } -void ShareSystem::BindNativeToPlugin(CPlugin *pPlugin, - sp_native_t *native, - uint32_t index, - NativeEntry *pEntry) +void ShareSystem::BindNativeToPlugin(CPlugin *pPlugin, sp_native_t *native, uint32_t index, + const Ref &pEntry) { /* Mark as bound... we do the rest next. */ native->status = SP_NATIVE_BOUND; - native->user = pEntry; - - /* Perform a bind. */ native->pfn = pEntry->func(); /* We don't bother with dependency crap if the owner is Core. */ @@ -345,14 +338,10 @@ void ShareSystem::BindNativeToPlugin(CPlugin *pPlugin, if ((native->flags & SP_NTVFLAG_OPTIONAL) == SP_NTVFLAG_OPTIONAL) { /* Only add if there is a valid owner. */ - if (pEntry->owner != NULL) - { + if (pEntry->owner) pEntry->owner->AddWeakRef(WeakNative(pPlugin, index)); - } else - { native->status = SP_NATIVE_UNBOUND; - } } /* Otherwise, we're a strong dependent and not a weak one */ else @@ -373,23 +362,15 @@ void ShareSystem::BindNativeToPlugin(CPlugin *pPlugin, } } -NativeEntry *ShareSystem::AddNativeToCache(CNativeOwner *pOwner, const sp_nativeinfo_t *ntv) +PassRef ShareSystem::AddNativeToCache(CNativeOwner *pOwner, const sp_nativeinfo_t *ntv) { - NativeEntry *pEntry; - - if ((pEntry = FindNative(ntv->name)) == NULL) - { - pEntry = new NativeEntry(pOwner, ntv); - m_NtvCache.insert(ntv->name, pEntry); - return pEntry; - } - - if (pEntry->owner != NULL) + NativeCache::Insert i = m_NtvCache.findForAdd(ntv->name); + if (i.found()) return NULL; - pEntry->owner = pOwner; - pEntry->native = NULL; - return pEntry; + Ref entry = Newborn(new Native(pOwner, ntv)); + m_NtvCache.insert(ntv->name, entry); + return entry; } FakeNative::~FakeNative() @@ -399,27 +380,27 @@ FakeNative::~FakeNative() void ShareSystem::ClearNativeFromCache(CNativeOwner *pOwner, const char *name) { - NativeEntry *pEntry; - - if ((pEntry = FindNative(name)) == NULL) + NativeCache::Result r = m_NtvCache.find(name); + if (!r.found()) return; - if (pEntry->owner != pOwner) + Ref entry(*r); + if (entry->owner != pOwner) return; - pEntry->fake = NULL; - pEntry->native = NULL; - pEntry->owner = NULL; + // Clear out the owner bit as a sanity measure. + entry->owner = NULL; + + m_NtvCache.remove(r); } -NativeEntry *ShareSystem::AddFakeNative(IPluginFunction *pFunc, const char *name, SPVM_FAKENATIVE_FUNC func) +PassRef ShareSystem::AddFakeNative(IPluginFunction *pFunc, const char *name, SPVM_FAKENATIVE_FUNC func) { - NativeEntry *pEntry; - - if ((pEntry = FindNative(name)) != NULL && pEntry->owner != NULL) + Ref entry(FindNative(name)); + if (entry) return NULL; - ke::AutoPtr fake(new FakeNative(name, pFunc)); + AutoPtr fake(new FakeNative(name, pFunc)); fake->gate = g_pSourcePawn2->CreateFakeNative(func, fake); if (!fake->gate) @@ -427,16 +408,10 @@ NativeEntry *ShareSystem::AddFakeNative(IPluginFunction *pFunc, const char *name CNativeOwner *owner = g_PluginSys.GetPluginByCtx(fake->ctx->GetContext()); - if (!pEntry) - { - pEntry = new NativeEntry(owner, fake.take()); - m_NtvCache.insert(name, pEntry); - } else { - pEntry->owner = owner; - pEntry->fake = fake.take(); - } + entry = Newborn(new Native(owner, fake.take())); + m_NtvCache.insert(name, entry); - return pEntry; + return entry; } void ShareSystem::AddCapabilityProvider(IExtension *myself, IFeatureProvider *provider, @@ -496,13 +471,10 @@ FeatureStatus ShareSystem::TestNative(IPluginRuntime *pRuntime, const char *name } } - NativeEntry *entry = FindNative(name); + Ref entry = FindNative(name); if (!entry) return FeatureStatus_Unknown; - if (entry->owner) - return FeatureStatus_Available; - return FeatureStatus_Unavailable; } diff --git a/core/logic/ShareSys.h b/core/logic/ShareSys.h index 543200f4..b9cc1aec 100644 --- a/core/logic/ShareSys.h +++ b/core/logic/ShareSys.h @@ -36,10 +36,12 @@ #include #include #include +#include #include #include #include #include "common_logic.h" +#include "Native.h" using namespace SourceHook; @@ -64,68 +66,8 @@ struct IfaceInfo }; class CNativeOwner; -struct NativeEntry; class CPlugin; -struct FakeNative -{ - FakeNative(const char *name, IPluginFunction *fun) - : name(name), - ctx(fun->GetParentContext()), - call(fun), - gate(NULL) - { - } - ~FakeNative(); - - ke::AString name; - IPluginContext *ctx; - IPluginFunction *call; - SPVM_NATIVE_FUNC gate; -}; - -struct NativeEntry -{ - NativeEntry(CNativeOwner *owner, const sp_nativeinfo_t *native) - : owner(owner), - native(native), - fake(NULL) - { - } - NativeEntry(CNativeOwner *owner, FakeNative *fake) - : owner(owner), - native(NULL), - fake(fake) - { - } - - CNativeOwner *owner; - const sp_nativeinfo_t *native; - ke::AutoPtr fake; - - SPVM_NATIVE_FUNC func() const - { - if (native) - return native->func; - if (fake) - return fake->gate; - return NULL; - } - const char *name() const - { - if (native) - return native->name; - if (fake) - return fake->name.chars(); - return NULL; - } - - static inline bool matches(const char *name, const NativeEntry *entry) - { - return strcmp(name, entry->name()) == 0; - } -}; - struct Capability { IExtension *ext; @@ -178,23 +120,22 @@ public: } public: void BindNativesToPlugin(CPlugin *pPlugin, bool bCoreOnly); - void BindNativeToPlugin(CPlugin *pPlugin, NativeEntry *pEntry); - NativeEntry *AddFakeNative(IPluginFunction *pFunc, const char *name, SPVM_FAKENATIVE_FUNC func); - NativeEntry *FindNative(const char *name); + void BindNativeToPlugin(CPlugin *pPlugin, const ke::Ref &pEntry); + ke::PassRef AddFakeNative(IPluginFunction *pFunc, const char *name, SPVM_FAKENATIVE_FUNC func); + ke::PassRef FindNative(const char *name); private: - NativeEntry *AddNativeToCache(CNativeOwner *pOwner, const sp_nativeinfo_t *ntv); + ke::PassRef AddNativeToCache(CNativeOwner *pOwner, const sp_nativeinfo_t *ntv); void ClearNativeFromCache(CNativeOwner *pOwner, const char *name); - void BindNativeToPlugin(CPlugin *pPlugin, - sp_native_t *ntv, - uint32_t index, - NativeEntry *pEntry); + void BindNativeToPlugin(CPlugin *pPlugin, sp_native_t *ntv, uint32_t index, const ke::Ref &pEntry); private: + typedef NameHashSet, Native> NativeCache; + List m_Interfaces; HandleType_t m_TypeRoot; IdentityToken_t m_IdentRoot; HandleType_t m_IfaceType; IdentityType_t m_CoreType; - NameHashSet m_NtvCache; + NativeCache m_NtvCache; StringHashMap m_caps; }; diff --git a/public/amtl/am-refcounting.h b/public/amtl/am-refcounting.h index f4d9ff77..6323dcf8 100644 --- a/public/amtl/am-refcounting.h +++ b/public/amtl/am-refcounting.h @@ -1,4 +1,4 @@ -// vim: set sts=8 ts=2 sw=2 tw=99 et: +// vim: set sts=8 ts=4 sw=4 tw=99 et: // // Copyright (C) 2013, David Anderson and AlliedModders LLC // All rights reserved. @@ -31,6 +31,7 @@ #define _include_amtl_refcounting_h_ #include +#include namespace ke { @@ -189,6 +190,11 @@ class Ref { AddRef(); } + Ref(Moveable other) + : thing_(other->thing_) + { + other->thing_ = NULL; + } template Ref(const Ref &other) : thing_(*other) @@ -255,6 +261,13 @@ class Ref return *this; } + Ref &operator =(Moveable other) { + Release(); + thing_ = other->thing_; + other->thing_ = NULL; + return *this; + } + private: void AddRef() { if (thing_) diff --git a/public/sm_namehashset.h b/public/sm_namehashset.h index 12cd7d6e..85edd1cc 100644 --- a/public/sm_namehashset.h +++ b/public/sm_namehashset.h @@ -107,6 +107,26 @@ public: typedef typename Internal::Insert Insert; typedef typename Internal::iterator iterator; + Result find(const char *aKey) + { + return table_.find(aKey); + } + + Insert findForAdd(const char *aKey) + { + return table_.findForAdd(aKey); + } + + void add(Insert &i, const T &value) + { + return table_.add(i, value); + } + + void add(Insert &i, ke::Moveable value) + { + return table_.add(i, value); + } + bool retrieve(const char *aKey, T *value) { CharsAndLength key(aKey); @@ -126,6 +146,15 @@ public: return table_.add(i, value); } + bool insert(const char *aKey, ke::Moveable value) + { + CharsAndLength key(aKey); + Insert i = table_.findForAdd(key); + if (i.found()) + return false; + return table_.add(i, value); + } + bool contains(const char *aKey) { CharsAndLength key(aKey); @@ -143,6 +172,11 @@ public: return true; } + void remove(Result &r) + { + table_.remove(r); + } + void clear() { table_.clear();