Skip to content

Commit

Permalink
[189] Mosquitto database corrupted on power-loss. (#206)
Browse files Browse the repository at this point in the history
Mosquitto database writes are not atomic and if power is lost during
a write the file will be permanently lost.  This commit makes writes as
atomic as possible.

Signed-off-by: Keegan Callin <kc@kcallin.net>
Bug: #189
  • Loading branch information
kcallin authored and ralight committed Aug 16, 2016
1 parent ba2de88 commit 7ba3f3d
Showing 1 changed file with 53 additions and 13 deletions.
66 changes: 53 additions & 13 deletions src/persist.c
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,6 @@ int mqtt3_db_backup(struct mosquitto_db *db, bool shutdown)
char err[256];
char *outfile = NULL;
int len;
#ifndef WIN32
int dir_fd;
#endif

if(!db || !db->config || !db->config->persistence_filepath) return MOSQ_ERR_INVAL;
_mosquitto_log_printf(NULL, MOSQ_LOG_INFO, "Saving in-memory database to %s.", db->config->persistence_filepath);
Expand All @@ -367,6 +364,36 @@ int mqtt3_db_backup(struct mosquitto_db *db, bool shutdown)
}
snprintf(outfile, len, "%s.new", db->config->persistence_filepath);
outfile[len] = '\0';

#ifndef WIN32
/**
*
* If a system lost power during the rename operation at the
* end of this file the filesystem could potentially be left
* with a directory that looks like this after powerup:
*
* 24094 -rw-r--r-- 2 root root 4099 May 30 16:27 mosquitto.db
* 24094 -rw-r--r-- 2 root root 4099 May 30 16:27 mosquitto.db.new
*
* The 24094 shows that mosquitto.db.new is hard-linked to the
* same file as mosquitto.db. If fopen(outfile, "wb") is naively
* called then mosquitto.db will be truncated and the database
* potentially corrupted.
*
* Any existing mosquitto.db.new file must be removed prior to
* opening to guarantee that it is not hard-linked to
* mosquitto.db.
*
*/
rc = unlink(outfile);
if (rc != 0) {
if (errno != ENOENT) {
_mosquitto_log_printf(NULL, MOSQ_LOG_INFO, "Error saving in-memory database, unable to remove %s.", outfile);
goto error;
}
}
#endif

db_fptr = _mosquitto_fopen(outfile, "wb");
if(db_fptr == NULL){
_mosquitto_log_printf(NULL, MOSQ_LOG_INFO, "Error saving in-memory database, unable to open %s for writing.", outfile);
Expand Down Expand Up @@ -401,17 +428,30 @@ int mqtt3_db_backup(struct mosquitto_db *db, bool shutdown)
mqtt3_db_subs_retain_write(db, db_fptr);

#ifndef WIN32
/**
*
* Closing a file does not guarantee that the contents are
* written to disk. Need to flush to send data from app to OS
* buffers, then fsync to deliver data from OS buffers to disk
* (as well as disk hardware permits).
*
* man close (http://linux.die.net/man/2/close, 2016-06-20):
*
* "successful close does not guarantee that the data has
* been successfully saved to disk, as the kernel defers
* writes. It is not common for a filesystem to flush
* the buffers when the stream is closed. If you need
* to be sure that the data is physically stored, use
* fsync(2). (It will depend on the disk hardware at this
* point."
*
* This guarantees that the new state file will not overwrite
* the old state file before its contents are valid.
*
*/

fflush(db_fptr);
fsync(fileno(db_fptr));

if(db->config->persistence_location){
dir_fd = open(db->config->persistence_location, O_RDONLY);
}else{
dir_fd = open(".", O_RDONLY);
}
if(dir_fd > 0){
fsync(dir_fd);
close(dir_fd);
}
#endif
fclose(db_fptr);

Expand Down

0 comments on commit 7ba3f3d

Please sign in to comment.