Skip to content

Commit e89f689

Browse files
authored
Avoid race condition when server table is refreshed (#5982)
WHile we had some file locking for getting the cache hash table and the data server table, the latter is being brought in under a different name and then there is a renaming. That section was not under file locking and the gmt_server_data.txt file can get clobbered by movie or batch.
1 parent 3c0d215 commit e89f689

File tree

1 file changed

+28
-6
lines changed

1 file changed

+28
-6
lines changed

src/gmt_remote.c

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ void gmtremote_lock_off (struct GMT_CTRL *GMT, struct LOCFILE_FP **P) {
687687

688688
/* Deal with hash values of cache/data files */
689689

690-
GMT_LOCAL int gmtremote_get_url (struct GMT_CTRL *GMT, char *url, char *file, char *orig, unsigned int index) {
690+
GMT_LOCAL int gmtremote_get_url (struct GMT_CTRL *GMT, char *url, char *file, char *orig, unsigned int index, bool do_lock) {
691691
bool turn_ctrl_C_off = false;
692692
int curl_err = 0, error = GMT_NOERROR;
693693
long time_spent;
@@ -704,13 +704,13 @@ GMT_LOCAL int gmtremote_get_url (struct GMT_CTRL *GMT, char *url, char *file, ch
704704
if (GMT->current.io.internet_error) return 1; /* Not able to use remote copying in this session */
705705

706706
/* Make a lock */
707-
if ((LF = gmtremote_lock_on (GMT, file)) == NULL)
707+
if (do_lock && (LF = gmtremote_lock_on (GMT, file)) == NULL)
708708
return 1;
709709

710710
/* If file locking held us up as another process was downloading the same file,
711711
* then that file should now be available. So we check again if it is before proceeding */
712712

713-
if (!access (file, F_OK))
713+
if (do_lock && !access (file, F_OK))
714714
goto unlocking1; /* Yes it was, unlock and return no error */
715715

716716
/* Initialize the curl session */
@@ -750,7 +750,7 @@ GMT_LOCAL int gmtremote_get_url (struct GMT_CTRL *GMT, char *url, char *file, ch
750750
unlocking1:
751751

752752
/* Remove lock file after successful download */
753-
gmtremote_lock_off (GMT, &LF);
753+
if (do_lock) gmtremote_lock_off (GMT, &LF);
754754

755755
if (turn_ctrl_C_off) gmtremote_turn_off_ctrl_C_check ();
756756

@@ -812,6 +812,7 @@ GMT_LOCAL int gmtremote_refresh (struct GMTAPI_CTRL *API, unsigned int index) {
812812
time_t mod_time, right_now = time (NULL); /* Unix time right now */
813813
char indexpath[PATH_MAX] = {""}, old_indexpath[PATH_MAX] = {""}, new_indexpath[PATH_MAX] = {""}, url[PATH_MAX] = {""};
814814
const char *index_file = (index == GMT_HASH_INDEX) ? GMT_HASH_SERVER_FILE : GMT_INFO_SERVER_FILE;
815+
struct LOCFILE_FP *LF = NULL;
815816
struct GMT_CTRL *GMT = API->GMT; /* Short hand */
816817

817818
if (GMT->current.io.refreshed[index]) return GMT_NOERROR; /* Already been here */
@@ -827,7 +828,7 @@ GMT_LOCAL int gmtremote_refresh (struct GMTAPI_CTRL *API, unsigned int index) {
827828
}
828829
snprintf (url, PATH_MAX, "%s/%s", gmt_dataserver_url (API), index_file);
829830
GMT_Report (API, GMT_MSG_DEBUG, "Download remote file %s for the first time\n", url);
830-
if (gmtremote_get_url (GMT, url, indexpath, NULL, index)) {
831+
if (gmtremote_get_url (GMT, url, indexpath, NULL, index, true)) {
831832
GMT_Report (API, GMT_MSG_INFORMATION, "Failed to get remote file %s\n", url);
832833
if (!access (indexpath, F_OK)) gmt_remove_file (GMT, indexpath); /* Remove index file just in case it got corrupted or zero size */
833834
GMT->current.setting.auto_download = GMT_NO_DOWNLOAD; /* Temporarily turn off auto download in this session only */
@@ -862,21 +863,39 @@ GMT_LOCAL int gmtremote_refresh (struct GMTAPI_CTRL *API, unsigned int index) {
862863
strcpy (old_indexpath, indexpath); /* Duplicate path name */
863864
strcat (old_indexpath, ".old"); /* Append .old to the copied path */
864865
snprintf (url, PATH_MAX, "%s/%s", gmt_dataserver_url (API), index_file); /* Set remote path to new index file */
865-
if (gmtremote_get_url (GMT, url, new_indexpath, indexpath, index)) { /* Get the new index file from server */
866+
867+
/* Here we will try to download a file */
868+
869+
/* Make a lock on the file */
870+
if ((LF = gmtremote_lock_on (GMT, (char *)new_indexpath)) == NULL)
871+
return 1;
872+
873+
/* If file locking held us up as another process was downloading the same file,
874+
* then that file should now be available. So we check again if it is before proceeding */
875+
876+
if (!access (new_indexpath, F_OK)) { /* Yes it was! Undo lock and return no error */
877+
gmtremote_lock_off (GMT, &LF); /* Remove lock file after successful download (unless query) */
878+
return GMT_NOERROR;
879+
}
880+
881+
if (gmtremote_get_url (GMT, url, new_indexpath, indexpath, index, false)) { /* Get the new index file from server */
866882
GMT_Report (API, GMT_MSG_DEBUG, "Failed to download %s - Internet troubles?\n", url);
867883
if (!access (new_indexpath, F_OK)) gmt_remove_file (GMT, new_indexpath); /* Remove index file just in case it got corrupted or zero size */
884+
gmtremote_lock_off (GMT, &LF);
868885
return 1; /* Unable to update the file (no Internet?) - skip the tests */
869886
}
870887
if (!access (old_indexpath, F_OK))
871888
remove (old_indexpath); /* Remove old index file if it exists */
872889
GMT_Report (API, GMT_MSG_DEBUG, "Rename %s to %s\n", indexpath, old_indexpath);
873890
if (gmt_rename_file (GMT, indexpath, old_indexpath, GMT_RENAME_FILE)) { /* Rename existing file to .old */
874891
GMT_Report (API, GMT_MSG_ERROR, "Failed to rename %s to %s.\n", indexpath, old_indexpath);
892+
gmtremote_lock_off (GMT, &LF);
875893
return 1;
876894
}
877895
GMT_Report (API, GMT_MSG_DEBUG, "Rename %s to %s\n", new_indexpath, indexpath);
878896
if (gmt_rename_file (GMT, new_indexpath, indexpath, GMT_RENAME_FILE)) { /* Rename newly copied file to existing file */
879897
GMT_Report (API, GMT_MSG_ERROR, "Failed to rename %s to %s.\n", new_indexpath, indexpath);
898+
gmtremote_lock_off (GMT, &LF);
880899
return 1;
881900
}
882901

@@ -887,6 +906,7 @@ GMT_LOCAL int gmtremote_refresh (struct GMTAPI_CTRL *API, unsigned int index) {
887906

888907
if ((N = gmtremote_hash_load (GMT, indexpath, &nN)) == 0) { /* Read in the new array of hash structs, will return 0 if mismatch of entries */
889908
gmt_remove_file (GMT, indexpath); /* Remove corrupted index file */
909+
gmtremote_lock_off (GMT, &LF);
890910
return 1;
891911
}
892912

@@ -930,6 +950,8 @@ GMT_LOCAL int gmtremote_refresh (struct GMTAPI_CTRL *API, unsigned int index) {
930950
}
931951
else
932952
GMT->current.io.new_data_list = true; /* Flag that we wish to delete datasets older than entries in this file */
953+
/* Remove lock file after successful download */
954+
gmtremote_lock_off (GMT, &LF);
933955
}
934956
else
935957
GMT_Report (API, GMT_MSG_DEBUG, "File %s less than 24 hours old, refresh is premature.\n", indexpath);

0 commit comments

Comments
 (0)