Control: tags -1 upstream fixed-upstream patch On 2023-11-09 08:49:50 +0100, Paul Gevers wrote: > Control: forwarded -1 https://github.com/viking-gps/viking/issues/202 > > Hi, > > Thanks. I have forwarded the issue upstream.
I've attached a patch. This is the upstream patch by Rob Norris https://github.com/viking-gps/viking/commit/21ecac2 which I ported for viking 1.10 in Debian 12: * 2 hunks were rejected in src/vikmapslayer.c due to changes in reindented code (I just had to re-add the "if"). * the compilation was failing due to undefined variable dr, so I just had to take the current code at this point, giving this simple change in src/vikmapslayer.c: + DownloadResult_t dr = DOWNLOAD_NOT_REQUIRED; if (need_download) { - DownloadResult_t dr = vik_map_source_download( MAPS_LAYER_NTH_TYPE(mdi->maptype), &(mdi->mapcoord), mdi->filename_buf, handle); + dr = vik_map_source_download( MAPS_LAYER_NTH_TYPE(mdi->maptype), &(mdi->mapcoord), mdi->filename_buf, handle); I've done several tests, and everything is now fine on my machine. -- Vincent Lefèvre <vinc...@vinc17.net> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
Description: avoid segmentation fault on map download when quitting Bug-Debian: https://bugs.debian.org/1055624 Forwarded: https://github.com/viking-gps/viking/issues/202 Index: viking-1.10/src/curl_download.c =================================================================== --- viking-1.10.orig/src/curl_download.c +++ viking-1.10/src/curl_download.c @@ -249,6 +249,8 @@ CURL_download_t curl_download_uri ( cons g_warning("%s: http response: %ld for uri %s", __FUNCTION__, response, uri); res = CURL_DOWNLOAD_ERROR; } + } else if (res == CURLE_ABORTED_BY_CALLBACK) { + res = CURL_DOWNLOAD_ABORTED; } else { g_warning ( "%s: curl error: %d for uri %s", __FUNCTION__, res, uri ); res = CURL_DOWNLOAD_ERROR; Index: viking-1.10/src/curl_download.h =================================================================== --- viking-1.10.orig/src/curl_download.h +++ viking-1.10/src/curl_download.h @@ -32,7 +32,8 @@ G_BEGIN_DECLS typedef enum { CURL_DOWNLOAD_NO_ERROR = 0, CURL_DOWNLOAD_NO_NEWER_FILE, - CURL_DOWNLOAD_ERROR + CURL_DOWNLOAD_ERROR, + CURL_DOWNLOAD_ABORTED } CURL_download_t; typedef struct { Index: viking-1.10/src/download.c =================================================================== --- viking-1.10.orig/src/download.c +++ viking-1.10/src/download.c @@ -401,7 +401,11 @@ static DownloadResult_t download( const DownloadResult_t result = DOWNLOAD_SUCCESS; - if (ret != CURL_DOWNLOAD_NO_ERROR && ret != CURL_DOWNLOAD_NO_NEWER_FILE) { + if (ret == CURL_DOWNLOAD_ABORTED) { + g_debug("%s: download aborted: curl_download_get_url=%d", __FUNCTION__, ret); + failure = TRUE; + result = DOWNLOAD_USER_ABORTED; + } else if (ret == CURL_DOWNLOAD_ERROR) { g_debug("%s: download failed: curl_download_get_url=%d", __FUNCTION__, ret); failure = TRUE; result = DOWNLOAD_HTTP_ERROR; Index: viking-1.10/src/download.h =================================================================== --- viking-1.10.orig/src/download.h +++ viking-1.10/src/download.h @@ -107,6 +107,7 @@ typedef enum { DOWNLOAD_CONTENT_ERROR = -1, DOWNLOAD_SUCCESS = 0, DOWNLOAD_NOT_REQUIRED = 1, // Also 'successful'. e.g. Because file already exists and no time checks used + DOWNLOAD_USER_ABORTED = 2, // Not an error, but not really successful either - no actual download completed } DownloadResult_t; /* TODO: convert to Glib */ Index: viking-1.10/src/vikdemlayer.c =================================================================== --- viking-1.10.orig/src/vikdemlayer.c +++ viking-1.10/src/vikdemlayer.c @@ -1348,6 +1348,7 @@ static void srtm_dem_download_thread ( D } case DOWNLOAD_SUCCESS: case DOWNLOAD_NOT_REQUIRED: + case DOWNLOAD_USER_ABORTED: default: break; } Index: viking-1.10/src/vikmapslayer.c =================================================================== --- viking-1.10.orig/src/vikmapslayer.c +++ viking-1.10/src/vikmapslayer.c @@ -359,6 +359,7 @@ void maps_layer_uninit () { vik_mutex_free ( rq_mutex ); g_hash_table_destroy ( requests ); + rq_mutex = NULL; } /****************************************/ @@ -447,10 +448,15 @@ gboolean req_hash_remove_id (gpointer k static void requests_clear ( guint maptype ) { - guint id = vik_map_source_get_uniq_id ( MAPS_LAYER_NTH_TYPE(maptype) ); - g_mutex_lock ( rq_mutex ); - g_hash_table_foreach_remove ( requests, req_hash_remove_id, GUINT_TO_POINTER(id) ); - g_mutex_unlock ( rq_mutex ); + // Ensure mutex (and therefore hash) is available + // as can become unavailable on program exit + // yet this function may still be called from existing threads + if ( rq_mutex ) { + guint id = vik_map_source_get_uniq_id ( MAPS_LAYER_NTH_TYPE(maptype) ); + g_mutex_lock ( rq_mutex ); + g_hash_table_foreach_remove ( requests, req_hash_remove_id, GUINT_TO_POINTER(id) ); + g_mutex_unlock ( rq_mutex ); + } } /** @@ -1669,13 +1675,18 @@ static gchar *create_request_string ( Ma static void mark_request_complete ( MapDownloadInfo *mdi, gint x, gint y ) { - gchar *request = create_request_string ( mdi, x, y ); - g_mutex_lock ( rq_mutex ); - (void)g_hash_table_remove ( requests, request ); - g_mutex_unlock ( rq_mutex ); - if ( vik_verbose ) - g_debug ( "%s: %s", __FUNCTION__, request ); - g_free ( request ); + // Ensure mutex (and therefore hash) is available + // as can become unavailable on program exit + // yet this function may still be called from existing threads + if ( rq_mutex ) { + gchar *request = create_request_string ( mdi, x, y ); + g_mutex_lock ( rq_mutex ); + (void)g_hash_table_remove ( requests, request ); + g_mutex_unlock ( rq_mutex ); + if ( vik_verbose ) + g_debug ( "%s: %s", __FUNCTION__, request ); + g_free ( request ); + } } static int map_download_thread ( MapDownloadInfo *mdi, gpointer threaddata ) @@ -1794,8 +1805,9 @@ static int map_download_thread ( MapDown mdi->mapcoord.x = x; mdi->mapcoord.y = y; + DownloadResult_t dr = DOWNLOAD_NOT_REQUIRED; if (need_download) { - DownloadResult_t dr = vik_map_source_download( MAPS_LAYER_NTH_TYPE(mdi->maptype), &(mdi->mapcoord), mdi->filename_buf, handle); + dr = vik_map_source_download( MAPS_LAYER_NTH_TYPE(mdi->maptype), &(mdi->mapcoord), mdi->filename_buf, handle); switch ( dr ) { case DOWNLOAD_PARAMETERS_ERROR: case DOWNLOAD_HTTP_ERROR: @@ -1816,6 +1828,8 @@ static int map_download_thread ( MapDown case DOWNLOAD_NOT_REQUIRED: need_download = FALSE; break; + case DOWNLOAD_USER_ABORTED: + break; default: break; } @@ -1823,17 +1837,23 @@ static int map_download_thread ( MapDown mark_request_complete ( mdi, x, y ); - g_mutex_lock(mdi->mutex); - if (remove_mem_cache) + // Avoid attempting to update mapcache when download aborted + // 1. Since no real change to track + // 2. more importantly, if the program is ending then the mapcache may have been removed + if ( dr != DOWNLOAD_USER_ABORTED ) { + + g_mutex_lock(mdi->mutex); + if (remove_mem_cache) a_mapcache_remove_all_shrinkfactors ( x, y, mdi->mapcoord.z, vik_map_source_get_uniq_id(MAPS_LAYER_NTH_TYPE(mdi->maptype)), mdi->mapcoord.scale, mdi->vml->filename ); - if (mdi->refresh_display && mdi->map_layer_alive) { - /* TODO: check if it's on visible area */ - if ( need_download ) { - vik_layer_emit_update ( VIK_LAYER(mdi->vml), FALSE ); // NB update display from background + if (mdi->refresh_display && mdi->map_layer_alive) { + /* TODO: check if it's on visible area */ + if ( need_download ) { + vik_layer_emit_update ( VIK_LAYER(mdi->vml), FALSE ); // NB update display from background + } } + g_mutex_unlock(mdi->mutex); + mdi->mapcoord.x = mdi->mapcoord.y = 0; /* we're temporarily between downloads */ } - g_mutex_unlock(mdi->mutex); - mdi->mapcoord.x = mdi->mapcoord.y = 0; /* we're temporarily between downloads */ } } }
