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

Replace boost::fibers::unbuffered_channel with boost::fibers::buffered_channel.

Using boost::fibers::unbuffered_channel can block the mainthread when calling mPendingCoprocs.push (LLCoprocedurePool::enqueueCoprocedure)
From the documentation:
- If a fiber attempts to send a value through an unbuffered channel and no fiber is waiting to receive the value, the channel will block the sending fiber.

This can happen if LLCoprocedurePool::coprocedureInvokerCoro is running a coroutine and this coroutine calls yield, resuming the viewers main loop. If inside
the main loop someone calls LLCoprocedurePool::enqueueCoprocedure now push will block, as there's no one waiting for a result right now.
The wait would be in LLCoprocedurePool::coprocedureInvokerCoro at the start of the while loop, but we have not reached that yet again as LLCoprocedurePool::coprocedureInvokerCoro
did yield before reaching pop_wait_for.
The result is a deadlock.

boost::fibers::buffered_channel will not block as long as there's space in the channel. A size of 4096 (DEFAULT_QUEUE_SIZE) should be plenty enough for this.
parent 32f1dfa5
No related branches found
No related tags found
No related merge requests found
...@@ -34,7 +34,7 @@ ...@@ -34,7 +34,7 @@
#include <chrono> #include <chrono>
#include <boost/assign.hpp> #include <boost/assign.hpp>
#include <boost/fiber/unbuffered_channel.hpp> #include <boost/fiber/buffered_channel.hpp>
#include "llexception.h" #include "llexception.h"
#include "stringize.h" #include "stringize.h"
...@@ -48,6 +48,7 @@ static const std::map<std::string, U32> DefaultPoolSizes{ ...@@ -48,6 +48,7 @@ static const std::map<std::string, U32> DefaultPoolSizes{
}; };
static const U32 DEFAULT_POOL_SIZE = 5; static const U32 DEFAULT_POOL_SIZE = 5;
static const U32 DEFAULT_QUEUE_SIZE = 4096;
//========================================================================= //=========================================================================
class LLCoprocedurePool: private boost::noncopyable class LLCoprocedurePool: private boost::noncopyable
...@@ -112,7 +113,7 @@ class LLCoprocedurePool: private boost::noncopyable ...@@ -112,7 +113,7 @@ class LLCoprocedurePool: private boost::noncopyable
// we use a deque here rather than std::queue since we want to be able to // we use a deque here rather than std::queue since we want to be able to
// iterate through the queue and potentially erase an entry from the middle. // iterate through the queue and potentially erase an entry from the middle.
typedef boost::fibers::unbuffered_channel<QueuedCoproc::ptr_t> CoprocQueue_t; typedef boost::fibers::buffered_channel<QueuedCoproc::ptr_t> CoprocQueue_t;
typedef std::map<LLUUID, LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t> ActiveCoproc_t; typedef std::map<LLUUID, LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t> ActiveCoproc_t;
std::string mPoolName; std::string mPoolName;
...@@ -283,7 +284,7 @@ void LLCoprocedureManager::close(const std::string &pool) ...@@ -283,7 +284,7 @@ void LLCoprocedureManager::close(const std::string &pool)
LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size):
mPoolName(poolName), mPoolName(poolName),
mPoolSize(size), mPoolSize(size),
mPendingCoprocs(), mPendingCoprocs(DEFAULT_QUEUE_SIZE),
//mShutdown(false), //mShutdown(false),
mCoroMapping(), mCoroMapping(),
mHTTPPolicy(LLCore::HttpRequest::DEFAULT_POLICY_ID) mHTTPPolicy(LLCore::HttpRequest::DEFAULT_POLICY_ID)
......
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