From bddafe5f7e4850ff04b788885dd4c16d365e6b97 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 2 Dec 2006 18:35:03 +0200 Subject: [PATCH] Document how timer callbacks erase timer IDs; add some assertions. Tangential to bug 868. --- src/bfu/dialog.c | 3 +++ src/bfu/leds.c | 3 +++ src/config/timer.c | 11 ++++++++++- src/document/refresh.c | 3 +++ src/ecmascript/ecmascript.c | 4 ++++ src/main/timer.c | 8 ++++++++ src/network/connection.c | 16 +++++++++++++++- src/network/progress.c | 7 +++++++ src/protocol/bittorrent/connection.c | 11 +++++++++-- src/protocol/bittorrent/peerconnect.c | 3 +++ src/protocol/bittorrent/tracker.c | 5 ++++- src/session/session.c | 6 ++++++ src/terminal/kbd.c | 3 +++ src/viewer/timer.c | 3 +++ 14 files changed, 81 insertions(+), 5 deletions(-) diff --git a/src/bfu/dialog.c b/src/bfu/dialog.c index 7795dd153..62048ef3c 100644 --- a/src/bfu/dialog.c +++ b/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 diff --git a/src/bfu/leds.c b/src/bfu/leds.c index 3ec76ef97..e54d99506 100644 --- a/src/bfu/leds.c +++ b/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; diff --git a/src/config/timer.c b/src/config/timer.c index 508cf6b39..0a7a9ceca 100644 --- a/src/config/timer.c +++ b/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 diff --git a/src/document/refresh.c b/src/document/refresh.c index e0493de36..69eb226b3 100644 --- a/src/document/refresh.c +++ b/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 diff --git a/src/ecmascript/ecmascript.c b/src/ecmascript/ecmascript.c index d2d1fa630..eeda6f0e5 100644 --- a/src/ecmascript/ecmascript.c +++ b/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); } diff --git a/src/main/timer.c b/src/main/timer.c index 7ecf21ab6..6aa912206 100644 --- a/src/main/timer.c +++ b/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) { diff --git a/src/network/connection.c b/src/network/connection.c index 7d8b73337..8ab89ac84 100644 --- a/src/network/connection.c +++ b/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 diff --git a/src/network/progress.c b/src/network/progress.c index 862115535..296476498 100644 --- a/src/network/progress.c +++ b/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) diff --git a/src/protocol/bittorrent/connection.c b/src/protocol/bittorrent/connection.c index 75f8c957b..619e125c2 100644 --- a/src/protocol/bittorrent/connection.c +++ b/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 diff --git a/src/protocol/bittorrent/peerconnect.c b/src/protocol/bittorrent/peerconnect.c index e314b3d04..ddb80375d 100644 --- a/src/protocol/bittorrent/peerconnect.c +++ b/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); } diff --git a/src/protocol/bittorrent/tracker.c b/src/protocol/bittorrent/tracker.c index b62628591..857f46c5e 100644 --- a/src/protocol/bittorrent/tracker.c +++ b/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. */ diff --git a/src/session/session.c b/src/session/session.c index 383b73745..06377bc9b 100644 --- a/src/session/session.c +++ b/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); diff --git a/src/terminal/kbd.c b/src/terminal/kbd.c index f0ccac6d2..eb6e1ae65 100644 --- a/src/terminal/kbd.c +++ b/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 */ diff --git a/src/viewer/timer.c b/src/viewer/timer.c index 6fb53590c..610d621b3 100644 --- a/src/viewer/timer.c +++ b/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) {