https://git.reactos.org/?p=reactos.git;a=commitdiff;h=e36b89addba874fc9aa898...
commit e36b89addba874fc9aa898a42e894ef37465cd30 Author: Ged Murphy gedmurphy@reactos.org AuthorDate: Tue Dec 5 22:13:02 2017 +0000
[SERVMAN]
- Avoid a potential race whereby the current service selection can change before the propsheet thread starts up - Cleanup the depends data, it doesn't need to be passed around the propsheet --- .../mscutils/servman/dependencies_tv1.c | 29 ++++----- .../mscutils/servman/dependencies_tv2.c | 24 +++---- base/applications/mscutils/servman/precomp.h | 17 +++-- base/applications/mscutils/servman/propsheet.c | 75 +++++++++++----------- .../mscutils/servman/propsheet_depends.c | 54 ++++++++-------- .../mscutils/servman/propsheet_recovery.c | 3 +- 6 files changed, 107 insertions(+), 95 deletions(-)
diff --git a/base/applications/mscutils/servman/dependencies_tv1.c b/base/applications/mscutils/servman/dependencies_tv1.c index 5e764e51db..27d376ea54 100644 --- a/base/applications/mscutils/servman/dependencies_tv1.c +++ b/base/applications/mscutils/servman/dependencies_tv1.c @@ -10,8 +10,7 @@ #include "precomp.h"
LPWSTR -TV1_GetDependants(PSERVICEPROPSHEET pDlgInfo, - SC_HANDLE hService) +TV1_GetDependants(SC_HANDLE hService) { LPQUERY_SERVICE_CONFIG lpServiceConfig; LPWSTR lpStr = NULL; @@ -80,7 +79,7 @@ TV1_GetDependants(PSERVICEPROPSHEET pDlgInfo, }
VOID -TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, +TV1_AddDependantsToTree(PDEPENDDATA pDependData, HTREEITEM hParent, LPWSTR lpServiceName) { @@ -103,7 +102,7 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, if (hService) { /* Get a list of service dependents */ - lpDependants = TV1_GetDependants(pDlgInfo, hService); + lpDependants = TV1_GetDependants(hService); if (lpDependants) { lpStr = lpDependants; @@ -127,7 +126,7 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, }
/* Add it */ - AddItemToTreeView(pDlgInfo->hDependsTreeView1, + AddItemToTreeView(pDependData->hDependsTreeView1, hParent, lpServiceConfig->lpDisplayName, lpStr, @@ -156,7 +155,7 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, /* Load the 'No dependencies' string */ AllocAndLoadString(&lpNoDepends, hInstance, IDS_NO_DEPENDS);
- AddItemToTreeView(pDlgInfo->hDependsTreeView1, + AddItemToTreeView(pDependData->hDependsTreeView1, NULL, lpNoDepends, NULL, @@ -166,7 +165,7 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, LocalFree(lpNoDepends);
/* Disable the window */ - EnableWindow(pDlgInfo->hDependsTreeView1, FALSE); + EnableWindow(pDependData->hDependsTreeView1, FALSE); } }
@@ -178,25 +177,25 @@ TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, }
BOOL -TV1_Initialize(PSERVICEPROPSHEET pDlgInfo, +TV1_Initialize(PDEPENDDATA pDependData, LPWSTR lpServiceName) { BOOL bRet = FALSE;
/* Associate the imagelist with TV1 */ - pDlgInfo->hDependsTreeView1 = GetDlgItem(pDlgInfo->hDependsWnd, IDC_DEPEND_TREE1); - if (!pDlgInfo->hDependsTreeView1) + pDependData->hDependsTreeView1 = GetDlgItem(pDependData->hDependsWnd, IDC_DEPEND_TREE1); + if (!pDependData->hDependsTreeView1) { - ImageList_Destroy(pDlgInfo->hDependsImageList); - pDlgInfo->hDependsImageList = NULL; + ImageList_Destroy(pDependData->hDependsImageList); + pDependData->hDependsImageList = NULL; return FALSE; } - (void)TreeView_SetImageList(pDlgInfo->hDependsTreeView1, - pDlgInfo->hDependsImageList, + (void)TreeView_SetImageList(pDependData->hDependsTreeView1, + pDependData->hDependsImageList, TVSIL_NORMAL);
/* Set the first items in the control */ - TV1_AddDependantsToTree(pDlgInfo, NULL, lpServiceName); + TV1_AddDependantsToTree(pDependData, NULL, lpServiceName);
return bRet; } diff --git a/base/applications/mscutils/servman/dependencies_tv2.c b/base/applications/mscutils/servman/dependencies_tv2.c index a0f37b07c1..6f23197c0a 100644 --- a/base/applications/mscutils/servman/dependencies_tv2.c +++ b/base/applications/mscutils/servman/dependencies_tv2.c @@ -118,7 +118,7 @@ TV2_GetDependants(LPWSTR lpServiceName, }
VOID -TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, +TV2_AddDependantsToTree(PDEPENDDATA pDependData, HTREEITEM hParent, LPWSTR lpServiceName) { @@ -138,7 +138,7 @@ TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, bHasChildren = TV2_HasDependantServices(lpServiceStatus[i].lpServiceName);
/* Add it */ - AddItemToTreeView(pDlgInfo->hDependsTreeView2, + AddItemToTreeView(pDependData->hDependsTreeView2, hParent, lpServiceStatus[i].lpDisplayName, lpServiceStatus[i].lpServiceName, @@ -158,7 +158,7 @@ TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, /* Load the 'No dependencies' string */ AllocAndLoadString(&lpNoDepends, hInstance, IDS_NO_DEPENDS);
- AddItemToTreeView(pDlgInfo->hDependsTreeView2, + AddItemToTreeView(pDependData->hDependsTreeView2, NULL, lpNoDepends, NULL, @@ -168,31 +168,31 @@ TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, LocalFree(lpNoDepends);
/* Disable the window */ - EnableWindow(pDlgInfo->hDependsTreeView2, FALSE); + EnableWindow(pDependData->hDependsTreeView2, FALSE); } } }
BOOL -TV2_Initialize(PSERVICEPROPSHEET pDlgInfo, +TV2_Initialize(PDEPENDDATA pDependData, LPWSTR lpServiceName) { BOOL bRet = FALSE;
/* Associate the imagelist with TV2 */ - pDlgInfo->hDependsTreeView2 = GetDlgItem(pDlgInfo->hDependsWnd, IDC_DEPEND_TREE2); - if (!pDlgInfo->hDependsTreeView2) + pDependData->hDependsTreeView2 = GetDlgItem(pDependData->hDependsWnd, IDC_DEPEND_TREE2); + if (!pDependData->hDependsTreeView2) { - ImageList_Destroy(pDlgInfo->hDependsImageList); - pDlgInfo->hDependsImageList = NULL; + ImageList_Destroy(pDependData->hDependsImageList); + pDependData->hDependsImageList = NULL; return FALSE; } - (void)TreeView_SetImageList(pDlgInfo->hDependsTreeView2, - pDlgInfo->hDependsImageList, + (void)TreeView_SetImageList(pDependData->hDependsTreeView2, + pDependData->hDependsImageList, TVSIL_NORMAL);
/* Set the first items in the control */ - TV2_AddDependantsToTree(pDlgInfo, NULL, lpServiceName); + TV2_AddDependantsToTree(pDependData, NULL, lpServiceName);
return bRet; } diff --git a/base/applications/mscutils/servman/precomp.h b/base/applications/mscutils/servman/precomp.h index 051634dc95..5ec27109f6 100644 --- a/base/applications/mscutils/servman/precomp.h +++ b/base/applications/mscutils/servman/precomp.h @@ -128,11 +128,18 @@ typedef struct _SERVICEPROPSHEET { PMAIN_WND_INFO Info; ENUM_SERVICE_STATUS_PROCESS *pService; + +} SERVICEPROPSHEET, *PSERVICEPROPSHEET; + +typedef struct _DEPENDDATA +{ + PSERVICEPROPSHEET pDlgInfo; HIMAGELIST hDependsImageList; HWND hDependsWnd; HWND hDependsTreeView1; HWND hDependsTreeView2; -} SERVICEPROPSHEET, *PSERVICEPROPSHEET; + +} DEPENDDATA, *PDEPENDDATA;
HTREEITEM AddItemToTreeView(HWND hTreeView, HTREEITEM hRoot, LPWSTR lpDisplayName, LPWSTR lpServiceName, ULONG serviceType, BOOL bHasChildren); @@ -147,12 +154,12 @@ LPWSTR DisplayName, LPWSTR ServiceList);
/* tv1_dependencies */ -BOOL TV1_Initialize(PSERVICEPROPSHEET pDlgInfo, LPWSTR lpServiceName); -VOID TV1_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, HTREEITEM hParent, LPWSTR lpServiceName); +BOOL TV1_Initialize(PDEPENDDATA pDependData, LPWSTR lpServiceName); +VOID TV1_AddDependantsToTree(PDEPENDDATA pDependData, HTREEITEM hParent, LPWSTR lpServiceName);
/* tv2_dependencies */ -BOOL TV2_Initialize(PSERVICEPROPSHEET pDlgInfo, LPWSTR lpServiceName); -VOID TV2_AddDependantsToTree(PSERVICEPROPSHEET pDlgInfo, HTREEITEM hParent, LPWSTR lpServiceName); +BOOL TV2_Initialize(PDEPENDDATA pDependData, LPWSTR lpServiceName); +VOID TV2_AddDependantsToTree(PDEPENDDATA pDependData, HTREEITEM hParent, LPWSTR lpServiceName); BOOL TV2_HasDependantServices(LPWSTR lpServiceName); LPENUM_SERVICE_STATUS TV2_GetDependants(LPWSTR lpServiceName, LPDWORD lpdwCount);
diff --git a/base/applications/mscutils/servman/propsheet.c b/base/applications/mscutils/servman/propsheet.c index 9557420ebb..bd0ec24510 100644 --- a/base/applications/mscutils/servman/propsheet.c +++ b/base/applications/mscutils/servman/propsheet.c @@ -3,7 +3,7 @@ * LICENSE: GPL - See COPYING in the top level directory * FILE: base/applications/mscutils/servman/propsheet.c * PURPOSE: Property dialog box message handler - * COPYRIGHT: Copyright 2006-2007 Ged Murphy gedmurphy@reactos.org + * COPYRIGHT: Copyright 2006-2017 Ged Murphy gedmurphy@reactos.org * */
@@ -29,8 +29,20 @@ InitPropSheetPage(PROPSHEETPAGE *psp, VOID OpenPropSheet(PMAIN_WND_INFO Info) { + PSERVICEPROPSHEET pServicePropSheet; HANDLE hThread; - hThread = (HANDLE)_beginthreadex(NULL, 0, &PropSheetThread, Info, 0, NULL); + + pServicePropSheet = HeapAlloc(ProcessHeap, + 0, + sizeof(*pServicePropSheet)); + if (!pServicePropSheet) return; + + /* Set the current service in this calling thread to avoid + * it being updated before the thread is up */ + pServicePropSheet->pService = Info->pCurrentService; + pServicePropSheet->Info = Info; + + hThread = (HANDLE)_beginthreadex(NULL, 0, &PropSheetThread, pServicePropSheet, 0, NULL); if (hThread) { CloseHandle(hThread); @@ -40,65 +52,54 @@ OpenPropSheet(PMAIN_WND_INFO Info)
unsigned int __stdcall PropSheetThread(void* Param) { + PSERVICEPROPSHEET pServicePropSheet; PROPSHEETHEADER psh; PROPSHEETPAGE psp[4]; - PSERVICEPROPSHEET pServicePropSheet; HWND hDlg = NULL; MSG Msg;
- PMAIN_WND_INFO Info = (PMAIN_WND_INFO)Param; + pServicePropSheet = (PSERVICEPROPSHEET)Param;
ZeroMemory(&psh, sizeof(PROPSHEETHEADER)); psh.dwSize = sizeof(PROPSHEETHEADER); psh.dwFlags = PSH_PROPSHEETPAGE | PSH_PROPTITLE | PSH_MODELESS; - psh.hwndParent = Info->hMainWnd; + psh.hwndParent = pServicePropSheet->Info->hMainWnd; psh.hInstance = hInstance; psh.hIcon = LoadIcon(hInstance, MAKEINTRESOURCE(IDI_SM_ICON)); - psh.pszCaption = Info->pCurrentService->lpDisplayName; + psh.pszCaption = pServicePropSheet->Info->pCurrentService->lpDisplayName; psh.nPages = sizeof(psp) / sizeof(PROPSHEETPAGE); psh.nStartPage = 0; psh.ppsp = psp;
+ /* Initialize the tabs */ + InitPropSheetPage(&psp[0], pServicePropSheet, IDD_DLG_GENERAL, GeneralPageProc); + InitPropSheetPage(&psp[1], pServicePropSheet, IDD_LOGON, LogonPageProc); + InitPropSheetPage(&psp[2], pServicePropSheet, IDD_RECOVERY, RecoveryPageProc); + InitPropSheetPage(&psp[3], pServicePropSheet, IDD_DLG_DEPEND, DependenciesPageProc);
- pServicePropSheet = HeapAlloc(ProcessHeap, - 0, - sizeof(*pServicePropSheet)); - if (pServicePropSheet) + hDlg = (HWND)PropertySheetW(&psh); + if (hDlg) { - /* Save current service, as it could change while the dialog is open */ - pServicePropSheet->pService = Info->pCurrentService; - pServicePropSheet->Info = Info; - - InitPropSheetPage(&psp[0], pServicePropSheet, IDD_DLG_GENERAL, GeneralPageProc); - InitPropSheetPage(&psp[1], pServicePropSheet, IDD_LOGON, LogonPageProc); - InitPropSheetPage(&psp[2], pServicePropSheet, IDD_RECOVERY, RecoveryPageProc); - InitPropSheetPage(&psp[3], pServicePropSheet, IDD_DLG_DEPEND, DependenciesPageProc); - - hDlg = (HWND)PropertySheetW(&psh); - if (hDlg) + /* Pump the message queue */ + while (GetMessageW(&Msg, NULL, 0, 0)) { - /* Pump the message queue */ - while (GetMessageW(&Msg, NULL, 0, 0)) + if (PropSheet_GetCurrentPageHwnd(hDlg) == NULL) { + /* The user hit the ok / cancel button, pull it down */ + EnableWindow(pServicePropSheet->Info->hMainWnd, TRUE); + DestroyWindow(hDlg); + }
- if (PropSheet_GetCurrentPageHwnd(hDlg) == NULL) - { - /* The user hit the ok / cancel button, pull it down */ - EnableWindow(Info->hMainWnd, TRUE); - DestroyWindow(hDlg); - } - - if (PropSheet_IsDialogMessage(hDlg, &Msg) != 0) - { - TranslateMessage(&Msg); - DispatchMessageW(&Msg); - } + if (PropSheet_IsDialogMessage(hDlg, &Msg) != 0) + { + TranslateMessage(&Msg); + DispatchMessageW(&Msg); } } - - HeapFree(GetProcessHeap(), 0, pServicePropSheet); }
+ HeapFree(GetProcessHeap(), 0, pServicePropSheet); + return (hDlg != NULL); }
diff --git a/base/applications/mscutils/servman/propsheet_depends.c b/base/applications/mscutils/servman/propsheet_depends.c index b32111700e..de1d6a5573 100644 --- a/base/applications/mscutils/servman/propsheet_depends.c +++ b/base/applications/mscutils/servman/propsheet_depends.c @@ -159,20 +159,20 @@ TreeView_GetItemText(HWND hTreeView, */
static VOID -InitDependPage(PSERVICEPROPSHEET pDlgInfo) +InitDependPage(PDEPENDDATA pDependData) { /* Initialize the image list */ - pDlgInfo->hDependsImageList = InitImageList(IDI_NODEPENDS, - IDI_DRIVER, - GetSystemMetrics(SM_CXSMICON), - GetSystemMetrics(SM_CXSMICON), - IMAGE_ICON); + pDependData->hDependsImageList = InitImageList(IDI_NODEPENDS, + IDI_DRIVER, + GetSystemMetrics(SM_CXSMICON), + GetSystemMetrics(SM_CXSMICON), + IMAGE_ICON);
/* Set the first tree view */ - TV1_Initialize(pDlgInfo, pDlgInfo->pService->lpServiceName); + TV1_Initialize(pDependData, pDependData->pDlgInfo->pService->lpServiceName);
/* Set the second tree view */ - TV2_Initialize(pDlgInfo, pDlgInfo->pService->lpServiceName); + TV2_Initialize(pDependData, pDependData->pDlgInfo->pService->lpServiceName); }
/* @@ -185,12 +185,13 @@ DependenciesPageProc(HWND hwndDlg, WPARAM wParam, LPARAM lParam) { - PSERVICEPROPSHEET pDlgInfo; + + PDEPENDDATA pDependData;
/* Get the window context */ - pDlgInfo = (PSERVICEPROPSHEET)GetWindowLongPtr(hwndDlg, - GWLP_USERDATA); - if (pDlgInfo == NULL && uMsg != WM_INITDIALOG) + pDependData = (PDEPENDDATA)GetWindowLongPtr(hwndDlg, + GWLP_USERDATA); + if (pDependData == NULL && uMsg != WM_INITDIALOG) { return FALSE; } @@ -199,16 +200,17 @@ DependenciesPageProc(HWND hwndDlg, { case WM_INITDIALOG: { - pDlgInfo = (PSERVICEPROPSHEET)(((LPPROPSHEETPAGE)lParam)->lParam); - if (pDlgInfo != NULL) + pDependData = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(DEPENDDATA)); + if (pDependData != NULL) { SetWindowLongPtr(hwndDlg, GWLP_USERDATA, - (LONG_PTR)pDlgInfo); + (LONG_PTR)pDependData);
- pDlgInfo->hDependsWnd = hwndDlg; + pDependData->pDlgInfo = (PSERVICEPROPSHEET)(((LPPROPSHEETPAGE)lParam)->lParam); + pDependData->hDependsWnd = hwndDlg;
- InitDependPage(pDlgInfo); + InitDependPage(pDependData); } } break; @@ -226,19 +228,19 @@ DependenciesPageProc(HWND hwndDlg, if (lpnmtv->hdr.idFrom == IDC_DEPEND_TREE1) { /* Has this node been expanded before */ - if (!TreeView_GetChild(pDlgInfo->hDependsTreeView1, lpnmtv->itemNew.hItem)) + if (!TreeView_GetChild(pDependData->hDependsTreeView1, lpnmtv->itemNew.hItem)) { /* It's not, add the children */ - TV1_AddDependantsToTree(pDlgInfo, lpnmtv->itemNew.hItem, (LPWSTR)lpnmtv->itemNew.lParam); + TV1_AddDependantsToTree(pDependData, lpnmtv->itemNew.hItem, (LPWSTR)lpnmtv->itemNew.lParam); } } else if (lpnmtv->hdr.idFrom == IDC_DEPEND_TREE2) { /* Has this node been expanded before */ - if (!TreeView_GetChild(pDlgInfo->hDependsTreeView2, lpnmtv->itemNew.hItem)) + if (!TreeView_GetChild(pDependData->hDependsTreeView2, lpnmtv->itemNew.hItem)) { /* It's not, add the children */ - TV2_AddDependantsToTree(pDlgInfo, lpnmtv->itemNew.hItem, (LPWSTR)lpnmtv->itemNew.lParam); + TV2_AddDependantsToTree(pDependData, lpnmtv->itemNew.hItem, (LPWSTR)lpnmtv->itemNew.lParam); } } } @@ -256,11 +258,13 @@ DependenciesPageProc(HWND hwndDlg, break;
case WM_DESTROY: - DestroyTreeView(pDlgInfo->hDependsTreeView1); - DestroyTreeView(pDlgInfo->hDependsTreeView2); + DestroyTreeView(pDependData->hDependsTreeView1); + DestroyTreeView(pDependData->hDependsTreeView2);
- if (pDlgInfo->hDependsImageList) - ImageList_Destroy(pDlgInfo->hDependsImageList); + if (pDependData->hDependsImageList) + ImageList_Destroy(pDependData->hDependsImageList); + + HeapFree(GetProcessHeap(), 0, pDependData); }
return FALSE; diff --git a/base/applications/mscutils/servman/propsheet_recovery.c b/base/applications/mscutils/servman/propsheet_recovery.c index 8e16122151..59205f24c0 100644 --- a/base/applications/mscutils/servman/propsheet_recovery.c +++ b/base/applications/mscutils/servman/propsheet_recovery.c @@ -170,7 +170,8 @@ ShowFailureActions( { WCHAR szBuffer[256]; PWSTR startPtr, endPtr; - INT i, index, id, length; + INT index, id, length; + DWORD i;
for (i = 0; i < min(pRecoveryData->pServiceFailure->cActions, 3); i++) {