[1/6] qmgr: Only have one BackgroundCopyManager per system. [take 2]
Dan Hipschman
dsh at linux.ucla.edu
Thu Mar 6 21:04:35 CST 2008
This is the same as yesterday except it puts the critical section inside
the BackgroundCopyManagerImpl object and it removes ref counting.
---
dlls/qmgr/enum_jobs.c | 4 ++
dlls/qmgr/qmgr.c | 57 +++++--------------------
dlls/qmgr/qmgr.h | 2 +-
dlls/qmgr/tests/qmgr.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 120 insertions(+), 49 deletions(-)
diff --git a/dlls/qmgr/enum_jobs.c b/dlls/qmgr/enum_jobs.c
index e35730a..85aca47 100644
--- a/dlls/qmgr/enum_jobs.c
+++ b/dlls/qmgr/enum_jobs.c
@@ -181,6 +181,8 @@ HRESULT EnumBackgroundCopyJobsConstructor(LPVOID *ppObj,
/* Create array of jobs */
This->indexJobs = 0;
+
+ EnterCriticalSection(&qmgr->cs);
This->numJobs = list_count(&qmgr->jobs);
if (0 < This->numJobs)
@@ -189,6 +191,7 @@ HRESULT EnumBackgroundCopyJobsConstructor(LPVOID *ppObj,
This->numJobs * sizeof *This->jobs);
if (!This->jobs)
{
+ LeaveCriticalSection(&qmgr->cs);
HeapFree(GetProcessHeap(), 0, This);
return E_OUTOFMEMORY;
}
@@ -203,6 +206,7 @@ HRESULT EnumBackgroundCopyJobsConstructor(LPVOID *ppObj,
IBackgroundCopyJob_AddRef(iJob);
This->jobs[i++] = iJob;
}
+ LeaveCriticalSection(&qmgr->cs);
*ppObj = &This->lpVtbl;
return S_OK;
diff --git a/dlls/qmgr/qmgr.c b/dlls/qmgr/qmgr.c
index fdfd30c..c6fbc42 100644
--- a/dlls/qmgr/qmgr.c
+++ b/dlls/qmgr/qmgr.c
@@ -1,7 +1,7 @@
/*
* Queue Manager (BITS) core functions
*
- * Copyright 2007 Google (Roy Shea)
+ * Copyright 2007, 2008 Google (Roy Shea, Dan Hipschman)
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -23,29 +23,11 @@
WINE_DEFAULT_DEBUG_CHANNEL(qmgr);
-/* Destructor for instances of background copy manager */
-static void BackgroundCopyManagerDestructor(BackgroundCopyManagerImpl *This)
-{
- BackgroundCopyJobImpl *job;
- TRACE("%p\n", This);
-
- LIST_FOR_EACH_ENTRY(job, &This->jobs, BackgroundCopyJobImpl, entryFromQmgr)
- job->lpVtbl->Release((IBackgroundCopyJob *) job);
-
- HeapFree(GetProcessHeap(), 0, This);
-}
-
/* Add a reference to the iface pointer */
static ULONG WINAPI BITS_IBackgroundCopyManager_AddRef(
IBackgroundCopyManager* iface)
{
- BackgroundCopyManagerImpl * This = (BackgroundCopyManagerImpl *)iface;
- ULONG ref;
-
- TRACE("\n");
-
- ref = InterlockedIncrement(&This->ref);
- return ref;
+ return 2;
}
/* Attempt to provide a new interface to interact with iface */
@@ -74,17 +56,7 @@ static HRESULT WINAPI BITS_IBackgroundCopyManager_QueryInterface(
static ULONG WINAPI BITS_IBackgroundCopyManager_Release(
IBackgroundCopyManager* iface)
{
- BackgroundCopyManagerImpl * This = (BackgroundCopyManagerImpl *)iface;
- ULONG ref;
-
- TRACE("\n");
-
- ref = InterlockedDecrement(&This->ref);
- if (ref == 0)
- {
- BackgroundCopyManagerDestructor(This);
- }
- return ref;
+ return 1;
}
/*** IBackgroundCopyManager interface methods ***/
@@ -109,7 +81,9 @@ static HRESULT WINAPI BITS_IBackgroundCopyManager_CreateJob(
/* Add a reference to the job to job list */
IBackgroundCopyJob_AddRef(*ppJob);
job = (BackgroundCopyJobImpl *) *ppJob;
+ EnterCriticalSection(&This->cs);
list_add_head(&This->jobs, &job->entryFromQmgr);
+ LeaveCriticalSection(&This->cs);
return S_OK;
}
@@ -153,23 +127,16 @@ static const IBackgroundCopyManagerVtbl BITS_IBackgroundCopyManager_Vtbl =
BITS_IBackgroundCopyManager_GetErrorDescription
};
+static BackgroundCopyManagerImpl globalMgr = {
+ &BITS_IBackgroundCopyManager_Vtbl,
+ { NULL, -1, 0, 0, 0, 0 },
+ LIST_INIT(globalMgr.jobs)
+};
+
/* Constructor for instances of background copy manager */
HRESULT BackgroundCopyManagerConstructor(IUnknown *pUnkOuter, LPVOID *ppObj)
{
- BackgroundCopyManagerImpl *This;
-
TRACE("(%p,%p)\n", pUnkOuter, ppObj);
-
- This = HeapAlloc(GetProcessHeap(), 0, sizeof(*This));
- if (!This)
- {
- return E_OUTOFMEMORY;
- }
-
- This->lpVtbl = &BITS_IBackgroundCopyManager_Vtbl;
- This->ref = 1;
- list_init(&This->jobs);
-
- *ppObj = &This->lpVtbl;
+ *ppObj = (IBackgroundCopyManager *) &globalMgr;
return S_OK;
}
diff --git a/dlls/qmgr/qmgr.h b/dlls/qmgr/qmgr.h
index f40b1a5..166ee40 100644
--- a/dlls/qmgr/qmgr.h
+++ b/dlls/qmgr/qmgr.h
@@ -78,7 +78,7 @@ typedef struct
typedef struct
{
const IBackgroundCopyManagerVtbl *lpVtbl;
- LONG ref;
+ CRITICAL_SECTION cs;
struct list jobs;
} BackgroundCopyManagerImpl;
diff --git a/dlls/qmgr/tests/qmgr.c b/dlls/qmgr/tests/qmgr.c
index 07af527..a641951 100644
--- a/dlls/qmgr/tests/qmgr.c
+++ b/dlls/qmgr/tests/qmgr.c
@@ -26,6 +26,8 @@
#include "initguid.h"
#include "bits.h"
+WCHAR progname[MAX_PATH];
+
static void
test_CreateInstance(void)
{
@@ -130,11 +132,109 @@ static void test_EnumJobs(void)
IBackgroundCopyManager_Release(manager);
}
+static void run_child(WCHAR *secret)
+{
+ static const WCHAR format[] = {'%','s',' ','q','m','g','r',' ','%','s', 0};
+ WCHAR cmdline[MAX_PATH];
+ PROCESS_INFORMATION info;
+ STARTUPINFOW startup;
+
+ memset(&startup, 0, sizeof startup);
+ startup.cb = sizeof startup;
+
+ wsprintfW(cmdline, format, progname, secret);
+ ok(CreateProcessW(NULL, cmdline, NULL, NULL, FALSE, 0L, NULL, NULL, &startup, &info), "CreateProcess\n");
+ winetest_wait_child_process(info.hProcess);
+ ok(CloseHandle(info.hProcess), "CloseHandle\n");
+ ok(CloseHandle(info.hThread), "CloseHandle\n");
+}
+
+static void do_child(const char *secretA)
+{
+ WCHAR secretW[MAX_PATH];
+ IBackgroundCopyManager *manager = NULL;
+ GUID id;
+ IBackgroundCopyJob *job;
+ HRESULT hres;
+ hres = CoCreateInstance(&CLSID_BackgroundCopyManager, NULL,
+ CLSCTX_LOCAL_SERVER, &IID_IBackgroundCopyManager,
+ (void **) &manager);
+ if(hres != S_OK)
+ {
+ skip("Unable to create bits instance required for test.\n");
+ return;
+ }
+
+ MultiByteToWideChar(CP_ACP, 0, secretA, -1, secretW, MAX_PATH);
+ hres = IBackgroundCopyManager_CreateJob(manager, secretW,
+ BG_JOB_TYPE_DOWNLOAD, &id, &job);
+ ok(hres == S_OK, "CreateJob in child process\n");
+ IBackgroundCopyJob_Release(job);
+ IBackgroundCopyManager_Release(manager);
+}
+
+static void test_globalness(void)
+{
+ static const WCHAR format[] = {'t','e','s','t','_','%','u', 0};
+ WCHAR secretName[MAX_PATH];
+ IEnumBackgroundCopyJobs* enumJobs;
+ IBackgroundCopyManager *manager = NULL;
+ HRESULT hres;
+ hres = CoCreateInstance(&CLSID_BackgroundCopyManager, NULL,
+ CLSCTX_LOCAL_SERVER, &IID_IBackgroundCopyManager,
+ (void **) &manager);
+ if(hres != S_OK)
+ {
+ skip("Unable to create bits instance required for test.\n");
+ return;
+ }
+
+ wsprintfW(secretName, format, GetTickCount());
+ run_child(secretName);
+
+ hres = IBackgroundCopyManager_EnumJobs(manager, 0, &enumJobs);
+ ok(hres == S_OK, "EnumJobs failed: %08x\n", hres);
+ if(hres != S_OK)
+ skip("Unable to create job enumerator.\n");
+ else
+ {
+ ULONG i, n;
+ IBackgroundCopyJob *job;
+ BOOL found = FALSE;
+
+ hres = IEnumBackgroundCopyJobs_GetCount(enumJobs, &n);
+ for (i = 0; i < n && !found; ++i)
+ {
+ LPWSTR name;
+ IEnumBackgroundCopyJobs_Next(enumJobs, 1, &job, NULL);
+ IBackgroundCopyJob_GetDisplayName(job, &name);
+ if (lstrcmpW(name, secretName) == 0)
+ found = TRUE;
+ CoTaskMemFree(name);
+ IBackgroundCopyJob_Release(job);
+ }
+ hres = IEnumBackgroundCopyJobs_Release(enumJobs);
+ ok(found, "Adding a job in another process failed\n");
+ }
+
+ IBackgroundCopyManager_Release(manager);
+}
+
START_TEST(qmgr)
{
+ char **argv;
+ int argc = winetest_get_mainargs(&argv);
+ MultiByteToWideChar(CP_ACP, 0, argv[0], -1, progname, MAX_PATH);
+
CoInitialize(NULL);
- test_CreateInstance();
- test_CreateJob();
- test_EnumJobs();
+ if (argc == 3)
+ do_child(argv[2]);
+ else
+ {
+ test_CreateInstance();
+ test_CreateJob();
+ test_EnumJobs();
+ test_globalness();
+ }
CoUninitialize();
}
More information about the wine-patches
mailing list