Permalink
Browse files

[189] Mosquitto database corrupted on power-loss.

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: eclipse#189
  • Loading branch information...
1 parent b6385d3 commit b7ac6c2815418ffa6c11900db905fe7354a4ffcd @kcallin committed Jul 6, 2016
Showing with 53 additions and 13 deletions.
  1. +53 −13 src/persist.c
View
@@ -350,9 +350,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);
@@ -365,6 +362,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);
@@ -399,17 +426,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);

0 comments on commit b7ac6c2

Please sign in to comment.