Fix a race condition when saving the storage on logout

The storage data is no longer cached in the char server. This removal
doesn't cause any performance loss, since the cached data was refreshed
(loaded from the database) after every save operation. This commit
moves the load at the beginning of the save rather than at the end, and
discars the cached copy after the save is completed.

Related to #1754

Signed-off-by: Haru <haru@dotalux.com>

3 files changed, 18 insertions, 82 deletions

diff --git a/src/char/char.c b/src/char/char.c
index 5a5a0d7f2..f6556073e 100644
--- a/src/char/char.c
+++ b/src/char/char.c
@@ -288,16 +288,12 @@ void char_set_char_offline(int char_id, int account_id)
 	else
 	{
 		struct mmo_charstatus* cp = (struct mmo_charstatus*) idb_get(chr->char_db_,char_id);
-		struct storage_data *stor = (struct storage_data *) idb_get(inter_storage->account_storage, account_id);
 
 		inter_guild->CharOffline(char_id, cp?cp->guild_id:-1);
 
 		if (cp)
 			idb_remove(chr->char_db_,char_id);
 
-		if (stor) /* Remove inter-storage data. */
-			inter_storage->delete_account_storage(account_id);
-
 		if( SQL_ERROR == SQL->Query(inter->sql_handle, "UPDATE `%s` SET `online`='0' WHERE `char_id`='%d' LIMIT 1", char_db, char_id) )
 			Sql_ShowDebug(inter->sql_handle);
 	}
diff --git a/src/char/int_storage.c b/src/char/int_storage.c
index a3222ff72..20e852c66 100644
--- a/src/char/int_storage.c
+++ b/src/char/int_storage.c
@@ -41,26 +41,29 @@ struct inter_storage_interface inter_storage_s;
 struct inter_storage_interface *inter_storage;
 
 /// Save storage data to sql
-int inter_storage_tosql(int account_id, struct storage_data *cp, const struct storage_data *p)
+int inter_storage_tosql(int account_id, const struct storage_data *p)
 {
 	int i = 0, j = 0;
 	bool matched_p[MAX_STORAGE] = { false };
 	int delete[MAX_STORAGE] = { 0 };
 	int total_deletes = 0, total_updates = 0, total_inserts = 0;
 	int total_changes = 0;
+	struct storage_data cp = { 0 };
 	StringBuf buf;
 
-	nullpo_ret(cp);
 	nullpo_ret(p);
 
+	VECTOR_INIT(cp.item);
+	inter_storage->fromsql(account_id, &cp);
+
 	StrBuf->Init(&buf);
 
-	if (VECTOR_LENGTH(cp->item) > 0) {
+	if (VECTOR_LENGTH(cp.item) > 0) {
 		/**
 		 * Compare and update items, if any.
 		 */
-		for (i = 0; i < VECTOR_LENGTH(cp->item); i++) {
-			struct item *cp_it = &VECTOR_INDEX(cp->item, i);
+		for (i = 0; i < VECTOR_LENGTH(cp.item); i++) {
+			struct item *cp_it = &VECTOR_INDEX(cp.item, i);
 			struct item *p_it = NULL;
 
 			ARR_FIND(0, VECTOR_LENGTH(p->item), j,
@@ -154,15 +157,10 @@ int inter_storage_tosql(int account_id, struct storage_data *cp, const struct st
 		Sql_ShowDebug(inter->sql_handle);
 
 	StrBuf->Destroy(&buf);
-
-	/* re-sync loaded data with current table data. */
-	VECTOR_CLEAR(cp->item);
-	inter_storage->fromsql(account_id, cp);
+	VECTOR_CLEAR(cp.item);
 
 	total_changes = total_inserts + total_updates + total_deletes;
-
 	ShowInfo("storage save complete - id: %d (total: %d)\n", account_id, total_changes);
-
 	return total_changes;
 }
 
@@ -304,49 +302,15 @@ int inter_storage_guild_storage_fromsql(int guild_id, struct guild_storage* p)
 	return 0;
 }
 
-/**
- * Ensures storage data entity for a character.
- * @see DBCreateData
- */
-static struct DBData inter_storage_ensure_account_storage(union DBKey key, va_list args)
-{
-	struct storage_data *stor = NULL;
-
-	CREATE(stor, struct storage_data, 1);
-
-	stor->save = false;
-	stor->aggregate = 0;
-
-	VECTOR_INIT(stor->item);
-
-	return DB->ptr2data(stor);
-}
-
-/**
- * Cleaning function called through db_destroy() for vectors in storage_data.
- */
-int inter_storage_clear_account_storage(union DBKey key, struct DBData *data, va_list args)
-{
-	struct storage_data *stor = DB->data2ptr(data);
-
-	VECTOR_CLEAR(stor->item);
-
-	return 0;
-}
-
 //---------------------------------------------------------
 // storage data initialize
 int inter_storage_sql_init(void)
 {
-	inter_storage->account_storage = idb_alloc(DB_OPT_RELEASE_DATA);
-
 	return 1;
 }
 // storage data finalize
 void inter_storage_sql_final(void)
 {
-	inter_storage->account_storage->destroy(inter_storage->account_storage, inter_storage->clear_account_storage);
-
 	return;
 }
 
@@ -364,19 +328,6 @@ int inter_storage_guild_storage_delete(int guild_id)
 	return 0;
 }
 
-/**
- * Delete storage from memory for given account_id.
- * @param  account_id     [in] account id
- */
-void inter_storage_delete_account_storage(int account_id)
-{
-	struct storage_data *stor = (struct storage_data *)idb_get(inter_storage->account_storage, account_id);
-	if (stor == NULL)
-		return;
-	VECTOR_CLEAR(stor->item);
-	idb_remove(inter_storage->account_storage, account_id);
-}
-
 //---------------------------------------------------------
 // packet from map server
 
@@ -447,14 +398,13 @@ int mapif_parse_AccountStorageLoad(int fd)
  */
 int mapif_account_storage_load(int fd, int account_id)
 {
-	struct storage_data *stor = NULL;
+	struct storage_data stor = { 0 };
 	int count = 0, i = 0, len = 0;
 
 	Assert_ret(account_id > 0);
 
-	stor = (struct storage_data *) idb_ensure(inter_storage->account_storage, account_id, inter_storage->ensure_account_storage);
-
-	count = inter_storage->fromsql(account_id, stor);
+	VECTOR_INIT(stor.item);
+	count = inter_storage->fromsql(account_id, &stor);
 
 	len = 8 + count * sizeof(struct item);
 
@@ -463,9 +413,11 @@ int mapif_account_storage_load(int fd, int account_id)
 	WFIFOW(fd, 2) = (uint16) len;
 	WFIFOL(fd, 4) = account_id;
 	for (i = 0; i < count; i++)
-		memcpy(WFIFOP(fd, 8 + i * sizeof(struct item)), &VECTOR_INDEX(stor->item, i), sizeof(struct item));
+		memcpy(WFIFOP(fd, 8 + i * sizeof(struct item)), &VECTOR_INDEX(stor.item, i), sizeof(struct item));
 	WFIFOSET(fd, len);
 
+	VECTOR_CLEAR(stor.item);
+
 	return 1;
 }
 
@@ -479,13 +431,11 @@ int mapif_parse_AccountStorageSave(int fd)
 {
 	int payload_size = RFIFOW(fd, 2) - 8, account_id = RFIFOL(fd, 4);
 	int i = 0, count = 0;
-	struct storage_data *cp_stor = NULL, p_stor = { 0 };
+	struct storage_data p_stor = { 0 };
 
 	Assert_ret(fd > 0);
 	Assert_ret(account_id > 0);
 
-	cp_stor = (struct storage_data *) idb_ensure(inter_storage->account_storage, account_id, inter_storage->ensure_account_storage);
-
 	count = payload_size/sizeof(struct item);
 
 	VECTOR_INIT(p_stor.item);
@@ -502,7 +452,7 @@ int mapif_parse_AccountStorageSave(int fd)
 		p_stor.aggregate = count;
 	}
 
-	inter_storage->tosql(account_id, cp_stor, &p_stor);
+	inter_storage->tosql(account_id, &p_stor);
 
 	VECTOR_CLEAR(p_stor.item);
 
@@ -795,8 +745,6 @@ void inter_storage_defaults(void)
 {
 	inter_storage = &inter_storage_s;
 
-	inter_storage->ensure_account_storage = inter_storage_ensure_account_storage;
-	inter_storage->clear_account_storage = inter_storage_clear_account_storage;
 	inter_storage->tosql = inter_storage_tosql;
 	inter_storage->fromsql = inter_storage_fromsql;
 	inter_storage->guild_storage_tosql = inter_storage_guild_storage_tosql;
@@ -804,7 +752,6 @@ void inter_storage_defaults(void)
 	inter_storage->sql_init = inter_storage_sql_init;
 	inter_storage->sql_final = inter_storage_sql_final;
 	inter_storage->delete_ = inter_storage_delete;
-	inter_storage->delete_account_storage = inter_storage_delete_account_storage;
 	inter_storage->guild_storage_delete = inter_storage_guild_storage_delete;
 	inter_storage->parse_frommap = inter_storage_parse_frommap;
 }
diff --git a/src/char/int_storage.h b/src/char/int_storage.h
index 746629421..46bb6910f 100644
--- a/src/char/int_storage.h
+++ b/src/char/int_storage.h
@@ -31,20 +31,13 @@ struct guild_storage;
  * inter_storage interface
  **/
 struct inter_storage_interface {
-	/* */
-	struct DBMap *account_storage;
-	/* */
-	/* */
-	struct DBData (*ensure_account_storage) (union DBKey key, va_list args);
-	int (*clear_account_storage) (union DBKey key, struct DBData *data, va_list args);
-	int (*tosql) (int account_id, struct storage_data *cp, const struct storage_data *p);
+	int (*tosql) (int account_id, const struct storage_data *p);
 	int (*fromsql) (int account_id, struct storage_data *p);
 	int (*guild_storage_tosql) (int guild_id, const struct guild_storage *p);
 	int (*guild_storage_fromsql) (int guild_id, struct guild_storage* p);
 	int (*sql_init) (void);
 	void (*sql_final) (void);
 	int (*delete_) (int account_id);
-	void (*delete_account_storage) (int account_id);
 	int (*guild_storage_delete) (int guild_id);
 	int (*parse_frommap) (int fd);
 };