[1/4] qmgr: Add critical sections for Jobs and Files.
Dan Hipschman
dsh at linux.ucla.edu
Tue Mar 11 18:12:36 CDT 2008
This uses one less critical section and combines two of the original patches,
"[2/5] qmgr: Add critical sections for BackgroundCopyJobs" and
"[3/5] qmgr: Add critical sections for BackgroundCopyFiles" sent yesterday.
BackgroundCopyFiles don't get their own, they use the Jobs, which controls the
file list and progress for both file and job. The job list and job states are
controlled by the BackgroundCopyManager's critical section. This is just a lot
simpler.
---
dlls/qmgr/enum_files.c | 3 +++
dlls/qmgr/file.c | 10 ++++++++--
dlls/qmgr/job.c | 33 ++++++++++++++++++++++-----------
dlls/qmgr/qmgr.c | 2 +-
dlls/qmgr/qmgr.h | 10 ++++++++--
5 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/dlls/qmgr/enum_files.c b/dlls/qmgr/enum_files.c
index 082b541..36afc33 100644
--- a/dlls/qmgr/enum_files.c
+++ b/dlls/qmgr/enum_files.c
@@ -186,6 +186,7 @@ HRESULT EnumBackgroundCopyFilesConstructor(LPVOID *ppObj, IBackgroundCopyJob* iC
/* Create array of files */
This->indexFiles = 0;
+ EnterCriticalSection(&job->cs);
This->numFiles = list_count(&job->files);
This->files = NULL;
if (This->numFiles > 0)
@@ -194,6 +195,7 @@ HRESULT EnumBackgroundCopyFilesConstructor(LPVOID *ppObj, IBackgroundCopyJob* iC
This->numFiles * sizeof This->files[0]);
if (!This->files)
{
+ LeaveCriticalSection(&job->cs);
HeapFree(GetProcessHeap(), 0, This);
return E_OUTOFMEMORY;
}
@@ -206,6 +208,7 @@ HRESULT EnumBackgroundCopyFilesConstructor(LPVOID *ppObj, IBackgroundCopyJob* iC
This->files[i] = (IBackgroundCopyFile *) file;
++i;
}
+ LeaveCriticalSection(&job->cs);
*ppObj = &This->lpVtbl;
return S_OK;
diff --git a/dlls/qmgr/file.c b/dlls/qmgr/file.c
index 93ffba4..d879905 100644
--- a/dlls/qmgr/file.c
+++ b/dlls/qmgr/file.c
@@ -25,6 +25,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(qmgr);
static void BackgroundCopyFileDestructor(BackgroundCopyFileImpl *This)
{
+ This->owner->lpVtbl->Release((IBackgroundCopyJob *) This->owner);
HeapFree(GetProcessHeap(), 0, This->info.LocalName);
HeapFree(GetProcessHeap(), 0, This->info.RemoteName);
HeapFree(GetProcessHeap(), 0, This);
@@ -105,9 +106,11 @@ static HRESULT WINAPI BITS_IBackgroundCopyFile_GetProgress(
{
BackgroundCopyFileImpl *This = (BackgroundCopyFileImpl *) iface;
+ EnterCriticalSection(&This->owner->cs);
pVal->BytesTotal = This->fileProgress.BytesTotal;
pVal->BytesTransferred = This->fileProgress.BytesTransferred;
pVal->Completed = This->fileProgress.Completed;
+ LeaveCriticalSection(&This->owner->cs);
return S_OK;
}
@@ -122,8 +125,9 @@ static const IBackgroundCopyFileVtbl BITS_IBackgroundCopyFile_Vtbl =
BITS_IBackgroundCopyFile_GetProgress
};
-HRESULT BackgroundCopyFileConstructor(LPCWSTR remoteName,
- LPCWSTR localName, LPVOID *ppObj)
+HRESULT BackgroundCopyFileConstructor(BackgroundCopyJobImpl *owner,
+ LPCWSTR remoteName, LPCWSTR localName,
+ LPVOID *ppObj)
{
BackgroundCopyFileImpl *This;
int n;
@@ -160,6 +164,8 @@ HRESULT BackgroundCopyFileConstructor(LPCWSTR remoteName,
This->fileProgress.BytesTotal = BG_SIZE_UNKNOWN;
This->fileProgress.BytesTransferred = 0;
This->fileProgress.Completed = FALSE;
+ This->owner = owner;
+ owner->lpVtbl->AddRef((IBackgroundCopyJob *) owner);
*ppObj = &This->lpVtbl;
return S_OK;
diff --git a/dlls/qmgr/job.c b/dlls/qmgr/job.c
index 8768027..6bc63f4 100644
--- a/dlls/qmgr/job.c
+++ b/dlls/qmgr/job.c
@@ -25,6 +25,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(qmgr);
static void BackgroundCopyJobDestructor(BackgroundCopyJobImpl *This)
{
+ DeleteCriticalSection(&This->cs);
HeapFree(GetProcessHeap(), 0, This->displayName);
HeapFree(GetProcessHeap(), 0, This);
}
@@ -88,15 +89,17 @@ static HRESULT WINAPI BITS_IBackgroundCopyJob_AddFile(
/* We should return E_INVALIDARG in these cases. */
FIXME("Check for valid filenames and supported protocols\n");
- res = BackgroundCopyFileConstructor(RemoteUrl, LocalName, (LPVOID *) &pFile);
+ res = BackgroundCopyFileConstructor(This, RemoteUrl, LocalName, (LPVOID *) &pFile);
if (res != S_OK)
return res;
/* Add a reference to the file to file list */
IBackgroundCopyFile_AddRef(pFile);
file = (BackgroundCopyFileImpl *) pFile;
+ EnterCriticalSection(&This->cs);
list_add_head(&This->files, &file->entryFromJob);
++This->jobProgress.FilesTotal;
+ LeaveCriticalSection(&This->cs);
return S_OK;
}
@@ -120,24 +123,26 @@ static HRESULT WINAPI BITS_IBackgroundCopyJob_Resume(
IBackgroundCopyJob* iface)
{
BackgroundCopyJobImpl *This = (BackgroundCopyJobImpl *) iface;
+ HRESULT rv = S_OK;
+ EnterCriticalSection(&globalMgr.cs);
if (This->state == BG_JOB_STATE_CANCELLED
|| This->state == BG_JOB_STATE_ACKNOWLEDGED)
{
- return BG_E_INVALID_STATE;
+ rv = BG_E_INVALID_STATE;
}
-
- if (This->jobProgress.FilesTransferred == This->jobProgress.FilesTotal)
- return BG_E_EMPTY;
-
- if (This->state == BG_JOB_STATE_CONNECTING
- || This->state == BG_JOB_STATE_TRANSFERRING)
+ else if (This->jobProgress.FilesTransferred == This->jobProgress.FilesTotal)
{
- return S_OK;
+ rv = BG_E_EMPTY;
}
+ else if (This->state != BG_JOB_STATE_CONNECTING
+ && This->state != BG_JOB_STATE_TRANSFERRING)
+ {
+ This->state = BG_JOB_STATE_QUEUED;
+ }
+ LeaveCriticalSection(&globalMgr.cs);
- This->state = BG_JOB_STATE_QUEUED;
- return S_OK;
+ return rv;
}
static HRESULT WINAPI BITS_IBackgroundCopyJob_Cancel(
@@ -185,10 +190,12 @@ static HRESULT WINAPI BITS_IBackgroundCopyJob_GetProgress(
if (!pVal)
return E_INVALIDARG;
+ EnterCriticalSection(&This->cs);
pVal->BytesTotal = This->jobProgress.BytesTotal;
pVal->BytesTransferred = This->jobProgress.BytesTransferred;
pVal->FilesTotal = This->jobProgress.FilesTotal;
pVal->FilesTransferred = This->jobProgress.FilesTransferred;
+ LeaveCriticalSection(&This->cs);
return S_OK;
}
@@ -210,6 +217,7 @@ static HRESULT WINAPI BITS_IBackgroundCopyJob_GetState(
if (!pVal)
return E_INVALIDARG;
+ /* Don't think we need a critical section for this */
*pVal = This->state;
return S_OK;
}
@@ -441,6 +449,7 @@ HRESULT BackgroundCopyJobConstructor(LPCWSTR displayName, BG_JOB_TYPE type,
return E_OUTOFMEMORY;
This->lpVtbl = &BITS_IBackgroundCopyJob_Vtbl;
+ InitializeCriticalSection(&This->cs);
This->ref = 1;
This->type = type;
@@ -448,6 +457,7 @@ HRESULT BackgroundCopyJobConstructor(LPCWSTR displayName, BG_JOB_TYPE type,
This->displayName = HeapAlloc(GetProcessHeap(), 0, n);
if (!This->displayName)
{
+ DeleteCriticalSection(&This->cs);
HeapFree(GetProcessHeap(), 0, This);
return E_OUTOFMEMORY;
}
@@ -456,6 +466,7 @@ HRESULT BackgroundCopyJobConstructor(LPCWSTR displayName, BG_JOB_TYPE type,
hr = CoCreateGuid(&This->jobId);
if (FAILED(hr))
{
+ DeleteCriticalSection(&This->cs);
HeapFree(GetProcessHeap(), 0, This->displayName);
HeapFree(GetProcessHeap(), 0, This);
return hr;
diff --git a/dlls/qmgr/qmgr.c b/dlls/qmgr/qmgr.c
index c6fbc42..b0a6013 100644
--- a/dlls/qmgr/qmgr.c
+++ b/dlls/qmgr/qmgr.c
@@ -127,7 +127,7 @@ static const IBackgroundCopyManagerVtbl BITS_IBackgroundCopyManager_Vtbl =
BITS_IBackgroundCopyManager_GetErrorDescription
};
-static BackgroundCopyManagerImpl globalMgr = {
+BackgroundCopyManagerImpl globalMgr = {
&BITS_IBackgroundCopyManager_Vtbl,
{ NULL, -1, 0, 0, 0, 0 },
LIST_INIT(globalMgr.jobs)
diff --git a/dlls/qmgr/qmgr.h b/dlls/qmgr/qmgr.h
index 166ee40..a584694 100644
--- a/dlls/qmgr/qmgr.h
+++ b/dlls/qmgr/qmgr.h
@@ -41,6 +41,8 @@ typedef struct
struct list files;
BG_JOB_PROGRESS jobProgress;
BG_JOB_STATE state;
+ /* Protects file list, and progress */
+ CRITICAL_SECTION cs;
struct list entryFromQmgr;
} BackgroundCopyJobImpl;
@@ -72,12 +74,14 @@ typedef struct
BG_FILE_INFO info;
BG_FILE_PROGRESS fileProgress;
struct list entryFromJob;
+ BackgroundCopyJobImpl *owner;
} BackgroundCopyFileImpl;
/* Background copy manager vtbl and related data */
typedef struct
{
const IBackgroundCopyManagerVtbl *lpVtbl;
+ /* Protects job list and job states */
CRITICAL_SECTION cs;
struct list jobs;
} BackgroundCopyManagerImpl;
@@ -88,14 +92,16 @@ typedef struct
} ClassFactoryImpl;
extern ClassFactoryImpl BITS_ClassFactory;
+extern BackgroundCopyManagerImpl globalMgr;
HRESULT BackgroundCopyManagerConstructor(IUnknown *pUnkOuter, LPVOID *ppObj);
HRESULT BackgroundCopyJobConstructor(LPCWSTR displayName, BG_JOB_TYPE type,
GUID *pJobId, LPVOID *ppObj);
HRESULT EnumBackgroundCopyJobsConstructor(LPVOID *ppObj,
IBackgroundCopyManager* copyManager);
-HRESULT BackgroundCopyFileConstructor(LPCWSTR remoteName,
- LPCWSTR localName, LPVOID *ppObj);
+HRESULT BackgroundCopyFileConstructor(BackgroundCopyJobImpl *owner,
+ LPCWSTR remoteName, LPCWSTR localName,
+ LPVOID *ppObj);
HRESULT EnumBackgroundCopyFilesConstructor(LPVOID *ppObj,
IBackgroundCopyJob* copyJob);
More information about the wine-patches
mailing list