From 803678a90033b54606337e033cff5393a92adb5a Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Fri, 21 Nov 2003 18:33:10 +0000 Subject: [PATCH] thread-and-timer-corrections-to-afscreds-20031121 Changes to afscreds to place the obtain tokens dialog into a separate thread to prevent blocking of the Windows Message queue. Requires utilization of mutex semaphores to protect credential data structures. Previous versions of afscreds would set/unset the timer event every time the program received an event indicating user activity including dragging the mouse across the systemtray icon. This resulted in extremely unpredictable behavior. Now the timer event is only turned off when it must be turned off; and turned on when it must be turned on. The result is a credential expiration check once a minute. --- src/WINNT/client_creds/afscreds.h | 13 +++++- src/WINNT/client_creds/creds.cpp | 10 ++++- src/WINNT/client_creds/creds.h | 7 +++- src/WINNT/client_creds/credstab.cpp | 64 ++++++++++++++++++++++++----- src/WINNT/client_creds/main.cpp | 41 +++++++++++------- src/WINNT/client_creds/mounttab.cpp | 1 + src/WINNT/client_creds/resource.h | 1 + src/WINNT/client_creds/shortcut.cpp | 20 +++++---- src/WINNT/client_creds/shortcut.h | 2 +- src/WINNT/client_creds/trayicon.cpp | 2 + src/WINNT/client_creds/window.cpp | 30 ++++++++++---- 11 files changed, 146 insertions(+), 45 deletions(-) diff --git a/src/WINNT/client_creds/afscreds.h b/src/WINNT/client_creds/afscreds.h index 422c8385c7..a2f7999c50 100644 --- a/src/WINNT/client_creds/afscreds.h +++ b/src/WINNT/client_creds/afscreds.h @@ -24,6 +24,15 @@ #include #include #include +#ifdef __cplusplus +extern "C" { +#endif +#include +#include +#include +#ifdef __cplusplus +} +#endif #include "resource.h" #include "checklist.h" #include "window.h" @@ -79,6 +88,8 @@ typedef struct BOOL fStartup; BOOL fIsWinNT; TCHAR szHelpFile[ MAX_PATH ]; + osi_mutex_t expirationCheckLock; + osi_mutex_t credsLock; } GLOBALS; extern GLOBALS g; @@ -89,7 +100,7 @@ extern GLOBALS g; * */ -#define cminREMIND_TEST 3 // test every minute for expired creds +#define cminREMIND_TEST 1 // test every minute for expired creds #define cminREMIND_WARN 15 // warn if creds expire in 15 minutes #define cmsecMOUSEOVER 1000 // retest freq when mouse is over tray icon diff --git a/src/WINNT/client_creds/creds.cpp b/src/WINNT/client_creds/creds.cpp index 0f3956c35d..994b73c8f9 100644 --- a/src/WINNT/client_creds/creds.cpp +++ b/src/WINNT/client_creds/creds.cpp @@ -177,10 +177,12 @@ BOOL IsServiceRunning (void) { QueryServiceStatus (hService, &Status); CloseServiceHandle (hService); - } + } else if ( IsDebuggerPresent() ) + OutputDebugString("Unable to open Transarc AFS Daemon Service\n"); CloseServiceHandle (hManager); - } + } else if ( IsDebuggerPresent() ) + OutputDebugString("Unable to open SC Manager\n"); return (Status.dwCurrentState == SERVICE_RUNNING); } @@ -251,6 +253,8 @@ int GetCurrentCredentials (void) { int rc = KTC_NOCM; + lock_ObtainMutex(&g.credsLock); + // Free any knowledge we currently have about the user's credentials // if (g.aCreds) @@ -324,6 +328,8 @@ int GetCurrentCredentials (void) } } + lock_ReleaseMutex(&g.credsLock); + // We've finished updating g.aCreds. Update the tray icon to reflect // whether the user currently has any credentials at all, and // re-enable the Remind timer. diff --git a/src/WINNT/client_creds/creds.h b/src/WINNT/client_creds/creds.h index 5d42ad7e19..ab87b647a6 100644 --- a/src/WINNT/client_creds/creds.h +++ b/src/WINNT/client_creds/creds.h @@ -10,6 +10,9 @@ #ifndef CREDS_H #define CREDS_H +#ifdef __cplusplus +extern "C" { +#endif /* * PROTOTYPES _________________________________________________________________ @@ -33,6 +36,8 @@ void GetGatewayName (LPTSTR pszGateway); BOOL Creds_OpenLibraries (void); void Creds_CloseLibraries (void); - +#ifdef __cplusplus +} +#endif #endif diff --git a/src/WINNT/client_creds/credstab.cpp b/src/WINNT/client_creds/credstab.cpp index dbcfe8241a..bbcb8a63e9 100644 --- a/src/WINNT/client_creds/credstab.cpp +++ b/src/WINNT/client_creds/credstab.cpp @@ -89,6 +89,7 @@ BOOL CALLBACK Creds_DlgProc (HWND hDlg, UINT msg, WPARAM wp, LPARAM lp) void Creds_OnCheckRemind (HWND hDlg) { LPTSTR pszCell = (LPTSTR)GetWindowLong (hDlg, DWL_USER); + lock_ObtainMutex(&g.credsLock); for (size_t iCreds = 0; iCreds < g.cCreds; ++iCreds) { if (!lstrcmpi (g.aCreds[ iCreds ].szCell, pszCell)) @@ -100,6 +101,7 @@ void Creds_OnCheckRemind (HWND hDlg) g.aCreds[ iCreds ].fRemind = IsDlgButtonChecked (hDlg, IDC_CREDS_REMIND); SaveRemind (iCreds); } + lock_ReleaseMutex(&g.credsLock); } @@ -115,6 +117,7 @@ void Creds_OnUpdate (HWND hDlg) return; } + lock_ObtainMutex(&g.credsLock); for (size_t iCreds = 0; iCreds < g.cCreds; ++iCreds) { if (!lstrcmpi (g.aCreds[ iCreds ].szCell, pszCell)) @@ -162,6 +165,7 @@ void Creds_OnUpdate (HWND hDlg) FreeString (pszCreds); } + lock_ReleaseMutex(&g.credsLock); CheckDlgButton (hDlg, IDC_CREDS_REMIND, (iCreds == g.cCreds) ? FALSE : g.aCreds[iCreds].fRemind); EnableWindow (GetDlgItem (hDlg, IDC_CREDS_OBTAIN), IsServiceRunning()); @@ -191,18 +195,41 @@ void Creds_OnClickDestroy (HWND hDlg) } +struct _obtaincreds { + DWORD type; + HWND parent; + char * cell; +}; + +void ObtainCredsThread(void * data) +{ + struct _obtaincreds * oc = (struct _obtaincreds *)data; + + ModalDialogParam (oc->type, oc->parent, (DLGPROC)NewCreds_DlgProc, (LPARAM)oc->cell); + free(oc->cell); + free(oc); +} + void ShowObtainCreds (BOOL fExpiring, LPTSTR pszCell) { - HWND hParent = (IsWindowVisible (g.hMain)) ? g.hMain : NULL; + struct _obtaincreds * oc = (struct _obtaincreds *)malloc(sizeof(struct _obtaincreds)); + if ( !oc ) + return; + oc->parent = (IsWindowVisible (g.hMain)) ? g.hMain : NULL; + oc->type = fExpiring ? IDD_NEWCREDS_EXPIRE : IDD_NEWCREDS; + oc->cell = _strdup(pszCell); - if (fExpiring) - { - ModalDialogParam (IDD_NEWCREDS_EXPIRE, hParent, (DLGPROC)NewCreds_DlgProc, (LPARAM)pszCell); - } - else // (!fExpiring) - { - ModalDialogParam (IDD_NEWCREDS, hParent, (DLGPROC)NewCreds_DlgProc, (LPARAM)pszCell); - } + HANDLE thread = 0; + ULONG threadID = 123; + + thread = CreateThread(NULL, 0, (PTHREAD_START_ROUTINE)ObtainCredsThread, + oc, 0, &threadID); + if (thread != NULL) + CloseHandle(thread); + else { + free(oc->cell); + free(oc); + } } @@ -217,7 +244,6 @@ BOOL CALLBACK NewCreds_DlgProc (HWND hDlg, UINT msg, WPARAM wp, LPARAM lp) case WM_DESTROY: InterlockedDecrement (&g.fShowingMessage); - Main_EnableRemindTimer (TRUE); break; case WM_COMMAND: @@ -278,6 +304,7 @@ void NewCreds_OnInitDialog (HWND hDlg) SetDlgItemText (hDlg, IDC_NEWCREDS_CELL, szCell); } + lock_ObtainMutex(&g.credsLock); for (size_t iCreds = 0; iCreds < g.cCreds; ++iCreds) { if (*pszCell && !lstrcmpi (g.aCreds[ iCreds ].szCell, pszCell)) @@ -292,8 +319,10 @@ void NewCreds_OnInitDialog (HWND hDlg) SetDlgItemText (hDlg, IDC_NEWCREDS_USER, g.aCreds[ iCreds ].szUser); PostMessage (hDlg, WM_NEXTDLGCTL, (WPARAM)GetDlgItem (hDlg, IDC_NEWCREDS_PASSWORD), TRUE); } + lock_ReleaseMutex(&g.credsLock); NewCreds_OnEnable (hDlg); + SetForegroundWindow(hDlg); KillTimer (g.hMain, ID_SERVICE_TIMER); } @@ -345,9 +374,21 @@ BOOL NewCreds_OnOK (HWND hDlg) void NewCreds_OnCancel (HWND hDlg) { - LPTSTR pszCell = (LPTSTR)GetWindowLong (hDlg, DWL_USER); + TCHAR szText[ cchRESOURCE ] = ""; + LPTSTR pszCell = NULL; + + if (GetDlgItem (hDlg, IDC_NEWCREDS_CELL)) + { + GetDlgItemText (hDlg, IDC_NEWCREDS_CELL, szText, cchRESOURCE); + if ( szText[0] ) + pszCell = szText; + } + + if ( !pszCell ) + pszCell = (LPTSTR)GetWindowLong (hDlg, DWL_USER); if (pszCell) { + lock_ObtainMutex(&g.credsLock); for (size_t iCreds = 0; iCreds < g.cCreds; ++iCreds) { if (!lstrcmpi (g.aCreds[ iCreds ].szCell, pszCell)) @@ -367,6 +408,7 @@ void NewCreds_OnCancel (HWND hDlg) } } } + lock_ReleaseMutex(&g.credsLock); } } diff --git a/src/WINNT/client_creds/main.cpp b/src/WINNT/client_creds/main.cpp index 2cafd0c90b..a4bc850455 100644 --- a/src/WINNT/client_creds/main.cpp +++ b/src/WINNT/client_creds/main.cpp @@ -127,18 +127,8 @@ BOOL InitApp (LPSTR pszCmdLineA) break; case 'x': case 'X': - DWORD LogonOption; - DWORD LSPtype, LSPsize; - HKEY NPKey; - LSPsize=sizeof(LogonOption); - (void) RegOpenKeyEx(HKEY_LOCAL_MACHINE, REG_CLIENT_PROVIDER_KEY, - 0, KEY_QUERY_VALUE, &NPKey); - RegQueryValueEx(NPKey, "LogonOptions", NULL, - &LSPtype, (LPBYTE)&LogonOption, &LSPsize); - RegCloseKey (NPKey); - if (ISHIGHSECURITY(LogonOption)) - DoMapShare(); - GlobalMountDrive(); + TestAndDoMapShare(SERVICE_START_PENDING); + TestAndDoMapShare(SERVICE_RUNNING); return 0; } @@ -225,6 +215,23 @@ BOOL InitApp (LPSTR pszCmdLineA) InitCommonControls(); RegisterCheckListClass(); + osi_Init(); + lock_InitializeMutex(&g.expirationCheckLock, "expiration check lock"); + lock_InitializeMutex(&g.credsLock, "global creds lock"); + + if ( IsDebuggerPresent() ) { + if ( !g.fIsWinNT ) + OutputDebugString("No Service Present on non-NT systems\n"); + else { + if ( IsServiceRunning() ) + OutputDebugString("AFSD Service started\n"); + else { + OutputDebugString("AFSD Service stopped\n"); + if ( !IsServiceConfigured() ) + OutputDebugString("AFSD Service not configured\n"); + } + } + } // Create a main window. All further initialization will be done during // processing of WM_INITDIALOG. @@ -253,12 +260,16 @@ BOOL InitApp (LPSTR pszCmdLineA) else if (!IsServerInstalled()) Message (MB_ICONHAND, IDS_UNCONFIG_TITLE, IDS_UNCONFIG_DESC); } - if (IsServiceRunning() && fShow) + if (IsServiceRunning()) { + if (fShow) { + if ( IsDebuggerPresent() ) + OutputDebugString("Displaying Main window\n"); Main_Show (TRUE); } - - return TRUE; + } else if ( IsDebuggerPresent() ) + OutputDebugString("Displaying Main window\n"); + return TRUE; } diff --git a/src/WINNT/client_creds/mounttab.cpp b/src/WINNT/client_creds/mounttab.cpp index 4694deb6be..b583ba4b3b 100644 --- a/src/WINNT/client_creds/mounttab.cpp +++ b/src/WINNT/client_creds/mounttab.cpp @@ -115,6 +115,7 @@ void Mount_OnInitDialog (HWND hDlg) void Mount_OnUpdate (HWND hDlg, BOOL fOnInitDialog) { DRIVEMAPLIST List; + memset(&List, 0, sizeof(DRIVEMAPLIST)); QueryDriveMapList (&List); HWND hList = GetDlgItem (hDlg, IDC_LIST); diff --git a/src/WINNT/client_creds/resource.h b/src/WINNT/client_creds/resource.h index 4cc3cc9db8..16b73ead4a 100644 --- a/src/WINNT/client_creds/resource.h +++ b/src/WINNT/client_creds/resource.h @@ -83,6 +83,7 @@ #define IDD_WIZ_FINISH 119 #define IDD_MAPPING 120 #define IDD_TERMINATE_SMALL_95 121 +#define IDD_AUTH 122 #define M_TERMINATE 3000 #define M_ACTIVATE 3001 #define M_REMIND 3002 diff --git a/src/WINNT/client_creds/shortcut.cpp b/src/WINNT/client_creds/shortcut.cpp index cb0123d0b3..bbb1a64fd6 100644 --- a/src/WINNT/client_creds/shortcut.cpp +++ b/src/WINNT/client_creds/shortcut.cpp @@ -16,6 +16,7 @@ extern "C" { #include #include #include +#undef INITGUID #include #include #include @@ -40,7 +41,7 @@ void Shortcut_Exit (void) } -BOOL Shortcut_Create (LPTSTR pszTarget, LPCTSTR pszSource, LPTSTR pszDesc) +BOOL Shortcut_Create (LPTSTR pszTarget, LPCTSTR pszSource, LPTSTR pszDesc, LPTSTR pszArgs) { IShellLink *psl; HRESULT rc = CoCreateInstance (CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, IID_IShellLink, (void **)&psl); @@ -56,13 +57,18 @@ BOOL Shortcut_Create (LPTSTR pszTarget, LPCTSTR pszSource, LPTSTR pszDesc) rc = psl->SetDescription (pszDesc ? pszDesc : pszSource); if (SUCCEEDED (rc)) { + if ( pszArgs ) + rc = psl->SetArguments (pszArgs); + if (SUCCEEDED (rc)) + { #ifdef UNICODE - rc = ppf->Save (pszTarget, TRUE); + rc = ppf->Save (pszTarget, TRUE); #else - WORD wsz[ MAX_PATH ]; - MultiByteToWideChar (CP_ACP, 0, pszTarget, -1, wsz, MAX_PATH); - rc = ppf->Save (wsz, TRUE); + WORD wsz[ MAX_PATH ]; + MultiByteToWideChar (CP_ACP, 0, pszTarget, -1, wsz, MAX_PATH); + rc = ppf->Save (wsz, TRUE); #endif + } } } ppf->Release (); @@ -75,7 +81,7 @@ BOOL Shortcut_Create (LPTSTR pszTarget, LPCTSTR pszSource, LPTSTR pszDesc) void Shortcut_FixStartup (LPCTSTR pszLinkName, BOOL fAutoStart) { - TCHAR szShortcut[ MAX_PATH ] = TEXT(""); + TCHAR szShortcut[ MAX_PATH + 10 ] = TEXT(""); HKEY hk; if (RegOpenKey (HKEY_LOCAL_MACHINE, TEXT("Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Shell Folders"), &hk) == 0) @@ -104,7 +110,7 @@ void Shortcut_FixStartup (LPCTSTR pszLinkName, BOOL fAutoStart) if (fAutoStart) { - Shortcut_Create (szShortcut, szSource); + Shortcut_Create (szShortcut, szSource, "Autostart Authentication Agent"); } else // (!g.fAutoStart) { diff --git a/src/WINNT/client_creds/shortcut.h b/src/WINNT/client_creds/shortcut.h index 12d165e670..ae82d5de91 100644 --- a/src/WINNT/client_creds/shortcut.h +++ b/src/WINNT/client_creds/shortcut.h @@ -18,7 +18,7 @@ void Shortcut_Init (void); void Shortcut_Exit (void); -BOOL Shortcut_Create (LPTSTR pszTarget, LPCTSTR pszSource, LPTSTR pszDesc = NULL); +BOOL Shortcut_Create (LPTSTR pszTarget, LPCTSTR pszSource, LPTSTR pszDesc = NULL, LPTSTR pszArgs = NULL); void Shortcut_FixStartup (LPCTSTR pszLinkName, BOOL fAutoStart); diff --git a/src/WINNT/client_creds/trayicon.cpp b/src/WINNT/client_creds/trayicon.cpp index 10a4c53c1d..c07a846c69 100644 --- a/src/WINNT/client_creds/trayicon.cpp +++ b/src/WINNT/client_creds/trayicon.cpp @@ -44,7 +44,9 @@ void ChangeTrayIcon (int nim) nid.uID = 0; nid.uFlags = NIF_ICON | NIF_MESSAGE | NIF_TIP; nid.uCallbackMessage = WM_TRAYICON; + lock_ObtainMutex(&g.credsLock); nid.hIcon = ((g.cCreds != 0) && (iExpired == (size_t)-1)) ? ICON_CREDS_YES : ICON_CREDS_NO; + lock_ReleaseMutex(&g.credsLock); GetString (nid.szTip, (g.fIsWinNT) ? IDS_TOOLTIP : IDS_TOOLTIP_95); Shell_NotifyIcon (nim, &nid); } diff --git a/src/WINNT/client_creds/window.cpp b/src/WINNT/client_creds/window.cpp index 35cf864d0f..f5e3577af0 100644 --- a/src/WINNT/client_creds/window.cpp +++ b/src/WINNT/client_creds/window.cpp @@ -162,11 +162,13 @@ BOOL CALLBACK Main_DlgProc (HWND hDlg, UINT msg, WPARAM wp, LPARAM lp) InsertMenu (hmDummy, 0, MF_POPUP, (UINT)hm, NULL); BOOL fRemind = FALSE; + lock_ObtainMutex(&g.credsLock); for (size_t iCreds = 0; iCreds < g.cCreds; ++iCreds) { if (g.aCreds[ iCreds ].fRemind) fRemind = TRUE; } + lock_ReleaseMutex(&g.credsLock); CheckMenuItem (hm, M_REMIND, MF_BYCOMMAND | ((fRemind) ? MF_CHECKED : MF_UNCHECKED)); TrackPopupMenu (GetSubMenu (hmDummy, 0), @@ -268,6 +270,7 @@ void Main_OnInitDialog (HWND hDlg) void Main_OnCheckMenuRemind (void) { BOOL fRemind = FALSE; + lock_ObtainMutex(&g.credsLock); for (size_t iCreds = 0; iCreds < g.cCreds; ++iCreds) { if (g.aCreds[ iCreds ].fRemind) @@ -283,6 +286,7 @@ void Main_OnCheckMenuRemind (void) SaveRemind (iCreds); } } + lock_ReleaseMutex(&g.credsLock); // Check the active tab, and fix its checkbox if necessary // @@ -297,7 +301,7 @@ void Main_OnCheckMenuRemind (void) // Make sure the reminder timer is going // - Main_EnableRemindTimer (TRUE); + Main_EnableRemindTimer (fRemind); } @@ -405,7 +409,6 @@ void Main_RepopulateTabs (BOOL fDestroyInvalid) if (IsWindowVisible (g.hMain)) fDestroyInvalid = FALSE; - Main_EnableRemindTimer (FALSE); // First we'll have to look around and see what credentials we currently // have. This call just updates g.aCreds[]; it doesn't do anything else. @@ -424,6 +427,7 @@ void Main_RepopulateTabs (BOOL fDestroyInvalid) size_t iTabOut = 0; size_t nCreds = 0; + lock_ObtainMutex(&g.credsLock); for (size_t iCreds = 0; iCreds < g.cCreds; ++iCreds) { if (g.aCreds[ iCreds ].szCell[0]) @@ -471,6 +475,7 @@ void Main_RepopulateTabs (BOOL fDestroyInvalid) } } } + lock_ReleaseMutex(&g.credsLock); if (REALLOC (aTabs, cTabs, 1+iTabOut, cREALLOC_TABS)) aTabs[ iTabOut++ ] = dwTABPARAM_MOUNT; @@ -533,7 +538,6 @@ void Main_RepopulateTabs (BOOL fDestroyInvalid) TabCtrl_SetCurSel (hTab, iTabSel); Main_OnSelectTab (); - Main_EnableRemindTimer (TRUE); fInHere = FALSE; } @@ -542,15 +546,23 @@ void Main_RepopulateTabs (BOOL fDestroyInvalid) void Main_EnableRemindTimer (BOOL fEnable) { - KillTimer (g.hMain, ID_REMIND_TIMER); + static BOOL bEnabled = FALSE; - if (fEnable) + if ( fEnable == FALSE && bEnabled == TRUE ) { + KillTimer (g.hMain, ID_REMIND_TIMER); + bEnabled = FALSE; + } else if ( fEnable == TRUE && bEnabled == FALSE ) { SetTimer (g.hMain, ID_REMIND_TIMER, (ULONG)cmsec1MINUTE * cminREMIND_TEST, NULL); + bEnabled = TRUE; + } } size_t Main_FindExpiredCreds (void) { + size_t retval = (size_t) -1; + lock_ObtainMutex(&g.expirationCheckLock); + lock_ObtainMutex(&g.credsLock); for (size_t iCreds = 0; iCreds < g.cCreds; ++iCreds) { if (!g.aCreds[ iCreds ].szCell[0]) @@ -574,10 +586,14 @@ size_t Main_FindExpiredCreds (void) llExpires /= c100ns1SECOND; if (llExpires <= (llNow + (LONGLONG)cminREMIND_WARN * csec1MINUTE)) - return iCreds; + retval = (size_t) iCreds; + break; } + + lock_ReleaseMutex(&g.credsLock); + lock_ReleaseMutex(&g.expirationCheckLock); - return (size_t)-1; + return retval; }