Skip to content
Snippets Groups Projects
Commit a601c559 authored by Nat Goodspeed's avatar Nat Goodspeed
Browse files

MAINT-5232: Ensure that llcoro::get_id() returns distinct values.

Until now, the "main coroutine" (the initial context) of each thread left
LLCoros::Current() NULL. The trouble with that is that llcoro::get_id()
returns that CoroData* as an opaque token, and we want distinct values for
every stack in the process. That would not be true if the "main coroutine" on
thread A returned the same value (NULL) as the "main coroutine" on thread B,
and so forth. Give each thread's "main coroutine" a dummy heap CoroData
instance of its own.
parent 976f4b62
No related branches found
No related tags found
No related merge requests found
...@@ -39,7 +39,12 @@ ...@@ -39,7 +39,12 @@
#include "llerror.h" #include "llerror.h"
#include "stringize.h" #include "stringize.h"
// do nothing, when we need nothing done namespace {
void no_op() {}
} // anonymous namespace
// Do nothing, when we need nothing done. This is a static member of LLCoros
// because CoroData is a private nested class.
void LLCoros::no_cleanup(CoroData*) {} void LLCoros::no_cleanup(CoroData*) {}
// CoroData for the currently-running coroutine. Use a thread_specific_ptr // CoroData for the currently-running coroutine. Use a thread_specific_ptr
...@@ -56,6 +61,35 @@ LLCoros::Current::Current() ...@@ -56,6 +61,35 @@ LLCoros::Current::Current()
// by LLCoros::mCoros. It merely identifies it. For this reason we // by LLCoros::mCoros. It merely identifies it. For this reason we
// instantiate it with a no-op cleanup function. // instantiate it with a no-op cleanup function.
static boost::thread_specific_ptr<LLCoros::CoroData> sCurrent(LLCoros::no_cleanup); static boost::thread_specific_ptr<LLCoros::CoroData> sCurrent(LLCoros::no_cleanup);
// If this is the first time we're accessing sCurrent for the running
// thread, its get() will be NULL. This could be a problem, in that
// llcoro::get_id() would return the same (NULL) token value for the "main
// coroutine" in every thread, whereas what we really want is a distinct
// value for every distinct stack in the process. So if get() is NULL,
// give it a heap CoroData: this ensures that llcoro::get_id() will return
// distinct values.
// This tactic is "leaky": sCurrent explicitly does not destroy any
// CoroData to which it points, and we do NOT enter these "main coroutine"
// CoroData instances in the LLCoros::mCoros map. They are dummy entries,
// and they will leak at process shutdown: one CoroData per thread.
if (! sCurrent.get())
{
// It's tempting to provide a distinct name for each thread's "main
// coroutine." But as getName() has always returned the empty string
// to mean "not in a coroutine," empty string should suffice here --
// and truthfully the additional (thread-safe!) machinery to ensure
// uniqueness just doesn't feel worth the trouble.
// We use a no-op callable and a minimal stack size because, although
// CoroData's constructor in fact initializes its mCoro with a
// coroutine with that stack size, no one ever actually enters it by
// calling mCoro().
sCurrent.reset(new CoroData(0, // no prev
"", // not a named coroutine
no_op, // no-op callable
1024)); // stacksize moot
}
mCurrent = &sCurrent; mCurrent = &sCurrent;
} }
...@@ -63,17 +97,21 @@ LLCoros::Current::Current() ...@@ -63,17 +97,21 @@ LLCoros::Current::Current()
LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller)
{ {
CoroData* current = Current(); CoroData* current = Current();
if (! current) // With the dummy CoroData set in LLCoros::Current::Current(), this
{ // pointer should never be NULL.
LL_ERRS("LLCoros") << "Calling " << caller << " from non-coroutine context!" << LL_ENDL; llassert_always(current);
}
return *current; return *current;
} }
//static //static
LLCoros::coro::self& LLCoros::get_self() LLCoros::coro::self& LLCoros::get_self()
{ {
return *get_CoroData("get_self()").mSelf; CoroData& current = get_CoroData("get_self()");
if (! current.mSelf)
{
LL_ERRS("LLCoros") << "Calling get_self() from non-coroutine context!" << LL_ENDL;
}
return *current.mSelf;
} }
//static //static
...@@ -224,13 +262,7 @@ bool LLCoros::kill(const std::string& name) ...@@ -224,13 +262,7 @@ bool LLCoros::kill(const std::string& name)
std::string LLCoros::getName() const std::string LLCoros::getName() const
{ {
CoroData* current = Current(); return Current()->mName;
if (! current)
{
// not in a coroutine
return "";
}
return current->mName;
} }
void LLCoros::setStackSize(S32 stacksize) void LLCoros::setStackSize(S32 stacksize)
......
...@@ -234,6 +234,7 @@ class LL_COMMON_API LLCoros: public LLSingleton<LLCoros> ...@@ -234,6 +234,7 @@ class LL_COMMON_API LLCoros: public LLSingleton<LLCoros>
Current(); Current();
operator LLCoros::CoroData*() { return get(); } operator LLCoros::CoroData*() { return get(); }
LLCoros::CoroData* operator->() { return get(); }
LLCoros::CoroData* get() { return mCurrent->get(); } LLCoros::CoroData* get() { return mCurrent->get(); }
void reset(LLCoros::CoroData* ptr) { mCurrent->reset(ptr); } void reset(LLCoros::CoroData* ptr) { mCurrent->reset(ptr); }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment