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

Save registry inside of scan transaction #2755

Merged
merged 1 commit into from
May 7, 2019

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented May 7, 2019

Problem

If you run a ckan scan command and Ctrl-C out of it at just the right time (towards the end, I had to add a WriteLine to get the timing correct), you can end up with a truncated registry.json file:

-rw-r--r-- 1 16933954 May  7 17:13  registry.json
-rw-r--r-- 1  9207808 May  7 17:11  registry.json_BROKEN

Anything you try to do after this will throw exceptions and generally not work.

Cause

RegistryManager.Save is transaction-aware:

public void Save(bool enforce_consistency = true)
{
TxFileManager file_transaction = new TxFileManager();
log.InfoFormat("Saving CKAN registry at {0}", path);
if (enforce_consistency)
{
// No saving the registry unless it's in a sane state.
registry.CheckSanity();
}
string directoryPath = Path.GetDirectoryName(path);
if (directoryPath == null)
{
log.ErrorFormat("Failed to save registry, invalid path: {0}", path);
throw new DirectoryNotFoundKraken(path, "Can't find a directory in " + path);
}
if (!Directory.Exists(directoryPath))
{
Directory.CreateDirectory(directoryPath);
}
file_transaction.WriteAllText(path, Serialize());
ExportInstalled(
Path.Combine(directoryPath, $"installed-{ksp.Name}.ckan"),
false, true
);
if (!Directory.Exists(ksp.InstallHistoryDir()))
{
Directory.CreateDirectory(ksp.InstallHistoryDir());
}
ExportInstalled(
Path.Combine(
ksp.InstallHistoryDir(),
$"installed-{ksp.Name}-{DateTime.Now.ToString("yyyy-MM-dd_HH-mm-ss")}.ckan"
),
false, true
);
}

... but KSP.ScanGameData calls it outside of its transaction:

CKAN/Core/KSP.cs

Lines 409 to 445 in f2209e2

public void ScanGameData()
{
var manager = RegistryManager.Instance(this);
using (TransactionScope tx = CkanTransaction.CreateTransactionScope())
{
manager.registry.ClearDlls();
// TODO: It would be great to optimise this to skip .git directories and the like.
// Yes, I keep my GameData in git.
// Alas, EnumerateFiles is *case-sensitive* in its pattern, which causes
// DLL files to be missed under Linux; we have to pick .dll, .DLL, or scanning
// GameData *twice*.
//
// The least evil is to walk it once, and filter it ourselves.
IEnumerable<string> files = Directory.EnumerateFiles(
GameData(),
"*",
SearchOption.AllDirectories
);
files = files.Where(file => Regex.IsMatch(file, @"\.dll$", RegexOptions.IgnoreCase));
foreach (string dll in files.Select(KSPPathUtils.NormalizePath))
{
var relativePath = ToRelativeGameDir(dll);
if (!DllIgnoreList.Contains(relativePath))
manager.registry.RegisterDll(this, dll);
}
manager.ScanDlc();
tx.Complete();
}
manager.Save(enforce_consistency: false);
}

When a TxFileManager is used outside of a transaction, it falls back to making live updates to the file system. So if you kill CKAN while it's doing that, it'll just stop after whatever byte happened to be most recently written.

Changes

Now KSP.ScanGameData saves the registry inside of its transaction. This way if you abort the operation, the original registry.json file will remain intact.

Fixes #2754.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix Core (ckan.dll) Issues affecting the core part of CKAN Pull request Registry Issues affecting the registry labels May 7, 2019
@Olympic1 Olympic1 removed the Bug Something is not working as intended label May 7, 2019
@HebaruSan HebaruSan added the Bug Something is not working as intended label May 7, 2019
@HebaruSan HebaruSan merged commit 8903e8e into KSP-CKAN:master May 7, 2019
@Olympic1 Olympic1 removed Bug Something is not working as intended Pull request labels May 7, 2019
@HebaruSan HebaruSan deleted the fix/scan-tx branch May 9, 2019 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Easy This is easy to fix Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aborting a ckan scan halfway through leaves a corrupt registry.json file
3 participants