Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

fix: remove a handle without fail other handles greater than the removed one #2016

Closed
wants to merge 1 commit into from

1 participant

ulion
ulion
Collaborator

currently, remove a handle will fail all other handles which greater than the removed one, this will happen when more than one plugindirectory script running within a overlapped time.
it's a obvious bug since the handle supposed not always be 0, there is possibility following example happens:

ui make a plugin dir listing request, which assigned handle 0:
plugin with handle 0 begin
json-rpc make another plugin dir listing request, which assigned handle 1:
plugin with handle 1 begin
plugin with handle 0 end, in the current code, the handle will be erased
plugin with handle 1 call function like setEndOfDirectory but handle 1 does not existed, the globalHandles vector size is 1 now the CPluginDirectory object with original handle 1 now on the handle 0 position, so the setEndOfDirectory call will failed, it's a mistake, this commit fix this problem.

ulion ulion closed this
ulion ulion deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
43 xbmc/filesystem/PluginDirectory.cpp
View
@@ -73,7 +73,23 @@ void CPluginDirectory::removeHandle(int handle)
{
CSingleLock lock(m_handleLock);
if (handle >= 0 && handle < (int)globalHandles.size())
- globalHandles.erase(globalHandles.begin() + handle);
+ {
+ globalHandles[handle] = NULL;
+ while (handle >= 0 and handle + 1 == (int)globalHandles.size() && NULL == globalHandles[handle])
+ {
+ globalHandles.erase(globalHandles.begin() + handle);
+ --handle;
+ }
+ }
+}
+
+// easy method to get dir object from handle.
+// lock only in the function maybe not strong enough, make sure lock before call it.
+CPluginDirectory *CPluginDirectory::dirFromHandle(int handle)
+{
+ if (handle >= 0 && handle < (int)globalHandles.size())
+ return globalHandles[handle];
+ return NULL;
}
bool CPluginDirectory::StartScript(const CStdString& strPath, bool retrievingDir)
@@ -161,13 +177,13 @@ bool CPluginDirectory::GetPluginResult(const CStdString& strPath, CFileItem &res
bool CPluginDirectory::AddItem(int handle, const CFileItem *item, int totalItems)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, " %s - called with an invalid handle.", __FUNCTION__);
return false;
}
- CPluginDirectory *dir = globalHandles[handle];
CFileItemPtr pItem(new CFileItem(*item));
dir->m_listItems->Add(pItem);
dir->m_totalItems = totalItems;
@@ -178,13 +194,13 @@ bool CPluginDirectory::AddItem(int handle, const CFileItem *item, int totalItems
bool CPluginDirectory::AddItems(int handle, const CFileItemList *items, int totalItems)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, " %s - called with an invalid handle.", __FUNCTION__);
return false;
}
- CPluginDirectory *dir = globalHandles[handle];
CFileItemList pItemList;
pItemList.Copy(*items);
dir->m_listItems->Append(pItemList);
@@ -196,12 +212,12 @@ bool CPluginDirectory::AddItems(int handle, const CFileItemList *items, int tota
void CPluginDirectory::EndOfDirectory(int handle, bool success, bool replaceListing, bool cacheToDisc)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, " %s - called with an invalid handle.", __FUNCTION__);
return;
}
- CPluginDirectory *dir = globalHandles[handle];
// set cache to disc
dir->m_listItems->SetCacheToDisc(cacheToDisc ? CFileItemList::CACHE_IF_SLOW : CFileItemList::CACHE_NEVER);
@@ -219,14 +235,13 @@ void CPluginDirectory::EndOfDirectory(int handle, bool success, bool replaceList
void CPluginDirectory::AddSortMethod(int handle, SORT_METHOD sortMethod, const CStdString &label2Mask)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, "%s - called with an invalid handle.", __FUNCTION__);
return;
}
- CPluginDirectory *dir = globalHandles[handle];
-
// TODO: Add all sort methods and fix which labels go on the right or left
switch(sortMethod)
{
@@ -556,12 +571,12 @@ bool CPluginDirectory::WaitOnScriptResult(const CStdString &scriptPath, int scri
void CPluginDirectory::SetResolvedUrl(int handle, bool success, const CFileItem *resultItem)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, " %s - called with an invalid handle.", __FUNCTION__);
return;
}
- CPluginDirectory* dir = globalHandles[handle];
dir->m_success = success;
*dir->m_fileResult = *resultItem;
@@ -613,13 +628,13 @@ void CPluginDirectory::SetContent(int handle, const CStdString &strContent)
void CPluginDirectory::SetProperty(int handle, const CStdString &strProperty, const CStdString &strValue)
{
CSingleLock lock(m_handleLock);
- if (handle < 0 || handle >= (int)globalHandles.size())
+ CPluginDirectory *dir = dirFromHandle(handle);
+ if (NULL == dir)
{
CLog::Log(LOGERROR, "%s called with an invalid handle.", __FUNCTION__);
return;
}
- CPluginDirectory *dir = globalHandles[handle];
if (strProperty == "fanart_image")
dir->m_listItems->SetArt("fanart", strValue);
else
1  xbmc/filesystem/PluginDirectory.h
View
@@ -71,6 +71,7 @@ class CPluginDirectory : public IDirectory
static std::vector<CPluginDirectory*> globalHandles;
static int getNewHandle(CPluginDirectory *cp);
static void removeHandle(int handle);
+ static CPluginDirectory *dirFromHandle(int handle);
static CCriticalSection m_handleLock;
CFileItemList* m_listItems;
Something went wrong with that request. Please try again.