Document how timer callbacks erase timer IDs; add some assertions.

Tangential to bug 868.

src/bfu/dialog.c

@@ -674,6 +674,8 @@ draw_dialog(struct dialog_data *dlg_data, int width, int height)
 #endif /* CONFIG_UTF8 */
 }
 
+/* Timer callback for @refresh->timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 do_refresh_dialog(struct dialog_data *dlg_data)
 {
@@ -700,6 +702,7 @@ do_refresh_dialog(struct dialog_data *dlg_data)
 
 	install_timer(&refresh->timer, RESOURCE_INFO_REFRESH,
 		      (void (*)(void *)) do_refresh_dialog, dlg_data);
+	/* The expired timer ID has now been erased.  */
 }
 
 void

src/bfu/leds.c

@@ -268,6 +268,8 @@ sync_leds(struct session *ses)
 	return 0;
 }
 
+/* Timer callback for @redraw_timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 redraw_leds(void *xxx)
 {
@@ -280,6 +282,7 @@ redraw_leds(void *xxx)
 	}
 
 	install_timer(&redraw_timer, LEDS_REFRESH_DELAY, redraw_leds, NULL);
+	/* The expired timer ID has now been erased.  */
 
 	if (drawing) return;
 	drawing = 1;

src/config/timer.c

@@ -18,6 +18,8 @@
 /* Timer for periodically saving configuration files to disk */
 static timer_id_T periodic_save_timer = TIMER_ID_UNDEF;
 
+/* Timer callback for @periodic_save_timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 periodic_save_handler(void *xxx)
 {
@@ -33,9 +35,16 @@ periodic_save_handler(void *xxx)
 		trigger_event(periodic_save_event_id);
 
 	interval = sec_to_ms(get_opt_int("infofiles.save_interval"));
-	if (!interval) return;
+	if (!interval) {
+		/* We should get here only if @periodic_save_handler
+		 * is being called from @periodic_save_change_hook or
+		 * @init_timer, rather than from the timer system.  */
+		assert(periodic_save_timer == TIMER_ID_UNDEF);
+		return;
+	}
 
 	install_timer(&periodic_save_timer, interval, periodic_save_handler, NULL);
+	/* The expired timer ID has now been erased.  */
 }
 
 static int

src/document/refresh.c

@@ -59,6 +59,8 @@ done_document_refresh(struct document_refresh *refresh)
 	mem_free(refresh);
 }
 
+/* Timer callback for @refresh->timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 do_document_refresh(void *data)
 {
@@ -69,6 +71,7 @@ do_document_refresh(void *data)
 	assert(refresh);
 
 	refresh->timer = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
 
 	/* When refreshing documents that will trigger a download (like
 	 * sourceforge's download pages) make sure that we do not endlessly

src/ecmascript/ecmascript.c

@@ -303,6 +303,9 @@ ecmascript_set_action(unsigned char **action, unsigned char *string)
 	}
 }
 
+/* Timer callback for @interpreter->vs->doc_view->document->timeout.
+ * As explained in @install_timer, this function must erase the
+ * expired timer ID from all variables.  */
 static void
 ecmascript_timeout_handler(void *i)
 {
@@ -310,6 +313,7 @@ ecmascript_timeout_handler(void *i)
 
 	assertm(interpreter->vs->doc_view, "setTimeout: vs with no document (e_f %d)", interpreter->vs->ecmascript_fragile);
 	interpreter->vs->doc_view->document->timeout = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
 
 	ecmascript_eval(interpreter, &interpreter->code, NULL);
 }

src/main/timer.c

@@ -58,6 +58,14 @@ check_timers(timeval_T *last_time)
 	timeval_copy(last_time, &now);
 }
 
+/* Install a timer that calls @func(@data) after @delay milliseconds.
+ * Store to *@id either the ID of the new timer, or TIMER_ID_UNDEF if
+ * the timer cannot be installed.  (This function ignores the previous
+ * value of *@id in any case.)
+ *
+ * When @func is called, the timer ID has become invalid.  @func
+ * should erase the expired timer ID from all variables, so that
+ * there's no chance it will be given to @kill_timer later.  */
 void
 install_timer(timer_id_T *id, milliseconds_T delay, void (*func)(void *), void *data)
 {

src/network/connection.c

@@ -327,10 +327,14 @@ update_connection_progress(struct connection *conn)
 	update_progress(conn->progress, conn->received, conn->est_length, conn->from);
 }
 
+/* Progress timer callback for @conn->progress.  As explained in
+ * @start_update_progress, this function must erase the expired timer
+ * ID from @conn->progress->timer.  */
 static void
 stat_timer(struct connection *conn)
 {
 	update_connection_progress(conn);
+	/* The expired timer ID has now been erased.  */
 	notify_connection_callbacks(conn);
 }
 
@@ -617,10 +621,13 @@ done:
 	register_check_queue();
 }
 
+/* Timer callback for @keepalive_timeout.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 keepalive_timer(void *x)
 {
 	keepalive_timeout = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
 	check_keepalive_connections();
 }
 
@@ -1127,14 +1134,20 @@ detach_connection(struct download *download, off_t pos)
 	free_entry_to(conn->cached, pos);
 }
 
+/* Timer callback for @conn->timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 connection_timeout(struct connection *conn)
 {
 	conn->timer = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
 	timeout_socket(conn->socket);
 }
 
-/* Huh, using two timers? Is this to account for changes of c->unrestartable
+/* Timer callback for @conn->timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.
+ *
+ * Huh, using two timers? Is this to account for changes of c->unrestartable
  * or can it be reduced? --jonas */
 static void
 connection_timeout_1(struct connection *conn)
@@ -1144,6 +1157,7 @@ connection_timeout_1(struct connection *conn)
 			 ? get_opt_int("connection.unrestartable_receive_timeout")
 			 : get_opt_int("connection.receive_timeout"))
 			* 500), (void (*)(void *)) connection_timeout, conn);
+	/* The expired timer ID has now been erased.  */
 }
 
 void

src/network/progress.c

@@ -41,9 +41,12 @@ init_progress(off_t start)
 void
 done_progress(struct progress *progress)
 {
+	assert(progress->timer == TIMER_ID_UNDEF);
 	mem_free(progress);
 }
 
+/* Called from the timer callback of @progress->timer.  This function
+ * erases the expired timer ID on behalf of the actual callback.  */
 void
 update_progress(struct progress *progress, off_t loaded, off_t size, off_t pos)
 {
@@ -87,8 +90,12 @@ update_progress(struct progress *progress, off_t loaded, off_t size, off_t pos)
 				   (progress->size - progress->pos) / progress->average_speed);
 
 	install_timer(&progress->timer, SPD_DISP_TIME, progress->timer_func, progress->timer_func_data);
+	/* The expired timer ID has now been erased.  */
 }
 
+/* As in @install_timer, @timer_func should erase the expired timer ID
+ * from @progress->timer.  The usual way to ensure this is to make
+ * @timer_func call @update_progress, which sets a new timer.  */
 void
 start_update_progress(struct progress *progress, void (*timer_func)(void *),
 		      void *timer_func_data)

src/protocol/bittorrent/connection.c

@@ -83,7 +83,10 @@ sort_bittorrent_peer_connections(struct bittorrent_connection *bittorrent)
 #endif
 }
 
-/* This is basically the choke period handler. */
+/* Timer callback for @bittorrent->timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.
+ *
+ * This is basically the choke period handler. */
 void
 update_bittorrent_connection_state(struct connection *conn)
 {
@@ -94,6 +97,7 @@ update_bittorrent_connection_state(struct connection *conn)
 	int max_uploads = get_opt_int("protocol.bittorrent.max_uploads");
 
 	set_bittorrent_connection_timer(conn);
+	/* The expired timer ID has now been erased.  */
 	set_connection_timeout(conn);
 
 	peer_conns = list_size(&bittorrent->peers);
@@ -182,6 +186,9 @@ update_bittorrent_connection_state(struct connection *conn)
 	}
 }
 
+/* Progress timer callback for @bittorrent->upload_progress.  As
+ * explained in @start_update_progress, this function must erase the
+ * expired timer ID from @bittorrent->upload_progress->timer.  */
 static void
 update_bittorrent_connection_upload(void *data)
 {
@@ -191,7 +198,7 @@ update_bittorrent_connection_upload(void *data)
 			bittorrent->uploaded,
 			bittorrent->downloaded,
 			bittorrent->uploaded);
-
+	/* The expired timer ID has now been erased.  */
 }
 
 void

src/protocol/bittorrent/peerconnect.c

@@ -102,12 +102,15 @@ check_bittorrent_peer_blacklisting(struct bittorrent_peer_connection *peer,
 /* Timeout scheduling: */
 /* ************************************************************************** */
 
+/* Timer callback for @peer->timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 bittorrent_peer_connection_timeout(struct bittorrent_peer_connection *peer)
 {
 	/* Unset the timer so it won't get stopped when removing the peer
 	 * connection. */
 	peer->timer = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
 
 	done_bittorrent_peer_connection(peer);
 }

src/protocol/bittorrent/tracker.c

@@ -130,7 +130,9 @@ check_bittorrent_stopped_request(void *____)
 	free_uri_list(&bittorrent_stopped_requests);
 }
 
-/* Called both when the timer expires and from the request sending front-end. */
+/* Timer callback for @bittorrent->tracker.timer.  As explained in
+ * @install_timer, this function must erase the expired timer ID from
+ * all variables.  Also called from the request sending front-end. */
 /* XXX: When called with event set to ``stopped'' failure handling should not
  * end the connection, since that is already in progress. */
 /* TODO: Make a special timer callback entry point that can check if
@@ -146,6 +148,7 @@ do_send_bittorrent_tracker_request(struct connection *conn)
 	int numwant, index, min_size;
 
 	bittorrent->tracker.timer = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
 
 	/* If the previous request didn't make it, nuke it. This shouldn't
 	 * happen but not doing this makes it a potential leak. */

src/session/session.c

@@ -144,6 +144,8 @@ done_saved_session_info(void)
 		done_session_info(session_info.next);
 }
 
+/* Timer callback for @info->timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 session_info_timeout(int id)
 {
@@ -151,6 +153,7 @@ session_info_timeout(int id)
 
 	if (!info) return;
 	info->timer = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
 	done_session_info(info);
 }
 
@@ -442,6 +445,8 @@ load_frames(struct session *ses, struct document_view *doc_view)
 	}
 }
 
+/* Timer callback for @ses->display_timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 void
 display_timer(struct session *ses)
 {
@@ -458,6 +463,7 @@ display_timer(struct session *ses)
 	install_timer(&ses->display_timer, t,
 		      (void (*)(void *)) display_timer,
 		      ses);
+	/* The expired timer ID has now been erased.  */
 
 	load_frames(ses, ses->doc_view);
 	load_css_imports(ses, ses->doc_view);

src/terminal/kbd.c

@@ -972,6 +972,8 @@ set_kbd_event(struct interlink_event *ev,
 	set_kbd_interlink_event(ev, key, modifier);
 }
 
+/* Timer callback for @itrm->timer.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 kbd_timeout(struct itrm *itrm)
 {
@@ -979,6 +981,7 @@ kbd_timeout(struct itrm *itrm)
 	int el;
 
 	itrm->timer = TIMER_ID_UNDEF;
+	/* The expired timer ID has now been erased.  */
 
 	assertm(itrm->in.queue.len, "timeout on empty queue");
 	assert(!itrm->blocked);	/* block_itrm should have killed itrm->timer */

src/viewer/timer.c

@@ -28,6 +28,8 @@ get_timer_duration(void)
 	return timer_duration;
 }
 
+/* Timer callback for @countdown.  As explained in @install_timer,
+ * this function must erase the expired timer ID from all variables.  */
 static void
 count_down(void *xxx)
 {
@@ -40,6 +42,7 @@ count_down(void *xxx)
 	} else {
 		countdown = TIMER_ID_UNDEF;
 	}
+	/* The expired timer ID has now been erased.  */
 
 	keybinding = kbd_nm_lookup(KEYMAP_MAIN, get_opt_str("ui.timer.action"));
 	if (keybinding) {