Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Retries to lock Services database #4085

Merged
merged 5 commits into from
Dec 16, 2022
Merged

Retries to lock Services database #4085

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b2d7f93
retries to lock database
DenisRumyantsev Dec 11, 2022
9deaf4a
Decrease timeout + Add retry message
KonstantinTyukalov Dec 13, 2022
c35ed95
Merge branch 'master' into users/DenisRumyantsev/retries-to-lock-data…
KonstantinTyukalov Dec 13, 2022
5aa4593
Merge branch 'master' into users/DenisRumyantsev/retries-to-lock-data…
KonstantinTyukalov Dec 16, 2022
79805cc
Merge branch 'master' into users/DenisRumyantsev/retries-to-lock-data…
KonstantinTyukalov Dec 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions src/Agent.Listener/Configuration.Windows/NativeWindowsServiceHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ public bool IsServiceExists(string serviceName)
ServiceController service = ServiceController.GetServices().FirstOrDefault(x => x.ServiceName.Equals(serviceName, StringComparison.OrdinalIgnoreCase));
return service != null;
}

public void InstallService(string serviceName, string serviceDisplayName, string logonAccount, string logonPassword, bool setServiceSidTypeAsUnrestricted)
{
Trace.Entering();
Expand Down Expand Up @@ -540,9 +540,27 @@ public void InstallService(string serviceName, string serviceDisplayName, string
failureActions.Add(new FailureAction(RecoverAction.Restart, 60000));

// Lock the Service Database
svcLock = LockServiceDatabase(scmHndl);
if (svcLock.ToInt64() <= 0)
int lockRetries = 10;
int retryTimeout = 5000;
while (true)
{
svcLock = LockServiceDatabase(scmHndl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LockServiceDatabase

[suggestion / not blocking]

According to the docs, LockServiceDatabase has no affect in Vista+ https://learn.microsoft.com/en-us/windows/win32/api/winsvc/nf-winsvc-lockservicedatabase

It may be necessary to use a Mutex instead to resolve the issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@merlynomsft thanks for heads up!
As far as I know, the LockServiceDatabase still returns the code indicates no other processes are trying to lock it, so in this case I think this fake "lock" works for agents since we're checking the return code each time.

As I see it may be dangerous when some other processes will try to register the services simultaneously with the agent, but it's a rare case I guess (at least we have not seen open issue about this)

Note we're thinking about moving the windows services code parts to the newer API and .net 6 (for now the .net 6 agent still uses .framework for windows services)

Let's see how to improve this in a separate issue/PR?
Thanks!


var svcLockIntCode = svcLock.ToInt64();
if (svcLockIntCode > 0)
{
break;
}

_term.WriteLine(StringUtil.Loc("ServiceLockErrorRetry", svcLockIntCode, retryTimeout / 1000));

lockRetries--;
if (lockRetries > 0)
{
Thread.Sleep(retryTimeout);
continue;
}

throw new InvalidOperationException(StringUtil.Loc("FailedToLockServiceDB"));
}

Expand Down Expand Up @@ -911,7 +929,7 @@ public void GrantDirectoryPermissionForAccount(string accountName, IList<string>
AddMemberToLocalGroup(accountName, groupName);

// grant permssion for folders
foreach(var folder in folders)
foreach (var folder in folders)
{
if (Directory.Exists(folder))
{
Expand All @@ -929,7 +947,7 @@ public void RevokeDirectoryPermissionForAccount(IList<string> folders)
Trace.Info(StringUtil.Format("Calculated unique group name {0}", groupName));

// remove the group from folders
foreach(var folder in folders)
foreach (var folder in folders)
{
if (Directory.Exists(folder))
{
Expand All @@ -938,7 +956,7 @@ public void RevokeDirectoryPermissionForAccount(IList<string> folders)
{
RemoveGroupFromFolderSecuritySetting(folder, groupName);
}
catch(Exception ex)
catch (Exception ex)
{
Trace.Error(ex);
}
Expand Down
1 change: 1 addition & 0 deletions src/Misc/layoutbin/en-US/strings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,7 @@
"ServiceConfigured": "Service {0} successfully configured",
"ServiceDelayedStartOptionSet": "Service {0} successfully set to delayed auto start",
"ServiceInstalled": "Service {0} successfully installed",
"ServiceLockErrorRetry": "Service DB lock failed with code {0}. Retrying after {1} seconds...",
"ServiceRecoveryOptionSet": "Service {0} successfully set recovery option",
"ServiceSidTypeSet": "Service {0} successfully set SID type",
"ServiceStartedSuccessfully": "Service {0} started successfully",
Expand Down