"Unable to parse settings.json" at startup after disk was full

[laanwj]

This was reported to me by someone who had a crash issue after the disk was full, ostensibly unable to write settings.json and ending up with a truncated file. The solution was obvious in retrospect: delete it. But not everyone is going to figure this out by themselves.

Should this error be fatal at all? Could it be ignored and logged? Or is a warning then deleting it better (at least in the GUI, that's harder to do for the daemon).

Another thing to look into might be to write the file atomically so that this doesn't occur.

[mat888]

So the settings.json file is becoming corrupted after the disk is full? That seems like a problem which should be avoided. I'm not sure deleting it, and removing any settings which may be important to the user should be the official solution.

[laanwj]

Yes, it is a problem that should be avoided (hence "Another thing to look into might be to write the file atomically so that this doesn't occur."), but even then corruption can still happen and there needs to be a way to recover. I do agree silently deleting it would not be a good thing. A dialog at startup to continue with "reset settings" might do the trick better.

[ryanofsky]

I think bitcoin-core/gui#379 should resolve this. It clarifies the error and adds an option in the gui to reset the file

[kristapsk]

So the settings.json file is becoming corrupted after the disk is full?

When disk is totally full on Linux, any file randomly can be truncated to 0 size (the one you will try to overwrite at that moment). Guess solution could be to rename old file to temporary name before writing the new one and deleting old one only after the new one is saved correctly?

[ryanofsky]

When disk is totally full on Linux, any file randomly can be truncated to 0 size (the one you will try to overwrite at that moment).

Interesting. Is this documented somewhere? It definitely suggests regardless of any write errors that the settings read code should be treating an empty file the same as a non-existent file, and just ignoring it instead of triggering a fatal error. (EDIT: I don't think this anymore, see below)

This is an easy change to make but I'd also be curious to hear from @laanwj if he knows whether the file was truncated to size 0.

Guess solution could be to rename old file to temporary name before writing the new one and deleting old one only after the new one is saved correctly?

I could be misunderstanding but I think this describes what happens currently:

bitcoin/src/util/system.cpp

Lines 572 to 579 in df2b5da

	if (!util::WriteSettings(path_tmp, m_settings.rw_settings, write_errors)) {
	SaveErrors(write_errors, errors);
	return false;
	}
	if (!RenameOver(path_tmp, path)) {
	SaveErrors({strprintf("Failed renaming settings file %s to %s\n", path_tmp.string(), path.string())}, errors);
	return false;
	}

, but maybe there could be additional syncs or checks added?

[kristapsk]

Interesting. Is this documented somewhere?

Haven't looked at the kernel code in detail for this, just my personal experience (had old blog written in PHP long long time ago where everything was saved in text files instead of proper database and file got truncated to zero bytes when partition ran out of space, IIRC code just read everything into memory, modified and then tried to overwrite old file).

[ryanofsky]

It definitely suggests regardless of any write errors that the settings read code should be treating an empty file the same as a non-existent file, and just ignoring it instead of triggering a fatal error.

Thinking about this more, I disagree with what I wrote earlier. Even if a settings file is truncated to size 0 and the old settings are completely lost (as opposed to partially corrupted), I still think the safest thing for bitcoind to do is refuse to start until the corrupt file is cleared away. It would unsafe to assume no important settings were saved before the corruption happened and just start anyway, because there could have been critical settings affecting security (sandbox settings) or privacy (tor settings, wallet settings) or performance (pruning, coins cache size) that would be important not to ignore before starting. So I don't think it would be good to treat a size 0 settings.json file differently than any other corrupt settings.json file.

I also don't know of a way of avoiding corruption when writing to a full disk that would work better then the current approach of renaming a new file over the old one. Maybe bitcoin should generally shutdown or avoid writing things if disk space gets too low, though.

I think since bitcoind is non-interactive, the best thing it can do on startup when settings are corrupt is show an error about the corruption, and exit like it is currently doing. Since bitcoin-qt is interactive, I think the change in bitcoin-core/gui#379 adding a dialog button to reset settings provides a decent way to recover there, and resolves this issue as well as we can for now