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.

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

[Bug] CKAN already running dialog appears even if PID in registry.locked is stale #3686

Closed
ghost opened this issue Oct 28, 2022 · 6 comments · Fixed by #3687
Closed

[Bug] CKAN already running dialog appears even if PID in registry.locked is stale #3686

ghost opened this issue Oct 28, 2022 · 6 comments · Fixed by #3687
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Linux Issues specific for Linux Mono Issues specific for Mono

Comments

@ghost
Copy link

ghost commented Oct 28, 2022

Problem

If you need to close CKAN with SIGKILL, the <GameDir>/CKAN/registry.locked file keeps the PID of your SIGKILL'ed PID of previous CKAN session, which results in a CKAN already running error on the next start.

Suggestions

Add a check if that PID in registry.locked exists and belongs to a CKAN process. If true show the already running error dialog, otherwise open CKAN without error dialog.

Describe alternatives you've considered

Additional context
Linux

@HebaruSan

This comment was marked as resolved.

@HebaruSan HebaruSan added Core (ckan.dll) Issues affecting the core part of CKAN Linux Issues specific for Linux Mono Issues specific for Mono labels Oct 28, 2022
@HebaruSan

This comment was marked as outdated.

@HebaruSan HebaruSan added the Bug Something is not working as intended label Oct 28, 2022
@HebaruSan
Copy link
Member

Yup, this line is bugged:

if (IsInstanceMaybeLocked(path))

That function takes the <KSP>/CKAN dir path as a parameter, but we're passing the path to the registry.json file.

private static string InstanceRegistryLockPath(string path)
{
return Path.Combine(path, "registry.locked");
}
public static bool IsInstanceMaybeLocked(string path)
{
return File.Exists(InstanceRegistryLockPath(path));
}

this.path = Path.Combine(path, "registry.json");

GameInstance.cs uses it correctly:

/// <returns>
/// true if the instance may be locked, false otherwise.
/// Note that this is a tentative value; if it's true,
/// we still need to try to acquire the lock to confirm it isn't stale.
/// NOTE: Will throw NotKSPDirKraken if the instance isn't valid!
/// Either be prepared to catch that exception, or check Valid first to avoid it.
/// </returns>
public bool IsMaybeLocked => RegistryManager.IsInstanceMaybeLocked(CkanDir());

@HebaruSan
Copy link
Member

Confirmed that fixing that problem fixes the overall issue:

1392 [1] DEBUG CKAN.RegistryManager (null) - Preparing to load registry at /home/user/.local/share/Steam/steamapps/common/Kerbal Space Program/CKAN
1393 [1] DEBUG CKAN.RegistryManager (null) - Checking for stale lock file at /home/user/.local/share/Steam/steamapps/common/Kerbal Space Program/CKAN/registry.locked
1393 [1] DEBUG CKAN.RegistryManager (null) - Lock file found at /home/user/.local/share/Steam/steamapps/common/Kerbal Space Program/CKAN/registry.locked
1393 [1] DEBUG CKAN.RegistryManager (null) - Lock file contents: 122202
1393 [1] DEBUG CKAN.RegistryManager (null) - Looking for process with ID: 122202
1394 [1] DEBUG CKAN.RegistryManager (null) - Deleting stale lock file at /home/user/.local/share/Steam/steamapps/common/Kerbal Space Program/CKAN/registry.locked
1394 [1] DEBUG CKAN.RegistryManager (null) - Trying to create lock file: /home/user/.local/share/Steam/steamapps/common/Kerbal Space Program/CKAN/registry.locked
1394 [1] DEBUG CKAN.RegistryManager (null) - Lock file created: /home/user/.local/share/Steam/steamapps/common/Kerbal Space Program/CKAN/registry.locked
1395 [1] DEBUG CKAN.RegistryManager (null) - Trying to load registry from /home/user/.local/share/Steam/steamapps/common/Kerbal Space Program/CKAN/registry.json

@HebaruSan HebaruSan changed the title [Feature] Show CKAN already running dialog only when PID in registry.locked is correct [Bug] CKAN already running dialog appears even if PID in registry.locked is stale Oct 28, 2022
@ghost
Copy link
Author

ghost commented Oct 28, 2022

Tested it on latest build, couldn't reproduce that behaviour. Thanks 👍

@ghost ghost closed this as completed Oct 28, 2022
@HebaruSan
Copy link
Member

Let's leave this issue open until the fix is merged. If we did a release from the master branch right now, the bug would still be present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Linux Issues specific for Linux Mono Issues specific for Mono
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant