Each time a new audio file is opened for streaming, we update the numplayed
and lastplayed fields of its afs info entry of the audio file table, then
update the global afsi and ahfi structures, then trigger an AFSI_CHANGE event,
which calls all table event handlers. The event handlers receive as arguments
a pointer to the old afsi and a pointer to the already modified aft row.
The AFSI_CHANGE event handler of the audio file table gets called in the
situation described above, and also via the touch and cpsi subcommands,
because these also change the afsi info structure and therefore trigger
AFSI_CHANGE events as well. The handler checks whether the affected audio
file is the current audio file, and if it is, extracts the modified afsi
from the audio file table into the global afsi structure.
The problem is that in case of the above "next audio file" scenario, the
old afsi pointer points to the global afsi structure, which gets updated
by the aft event handler. Therefore the event handlers which happen to run
after the event handler of the audio file table see the already updated afsi
through the old_afsi pointer.
Since the audio file table happens to be the first table of the 7-element
afs_tables[] array iterated by afs_event(), all six other event handlers see
the incorrect afsi. However, only the moods event handler looks at the old afsi
to update the mean and quadratic variation of the lastplayed and numplayed
values of the afs statistics. Due to the bug described above, the old and
new lastplayed and numplayed values coincide, and therefore the statistics
update is a no-op. In practice, at least for moods with many admissible files,
the values will only be slightly off, so the bug is rather benign.
Fix this by renaming the on-stack new_afsi to old_afsi, adjusting
initializations accordingly, and passing a pointer to this instance instead.
{
unsigned char file_hash[HASH2_SIZE];
struct osl_object afsi_obj;
- struct afs_info new_afsi;
+ struct afs_info old_afsi;
int ret;
struct afsi_change_event_data aced;
struct osl_object map, chunk_table_obj;
ret = -E_HASH_MISMATCH;
goto out;
}
- new_afsi = d->afsi;
- new_afsi.num_played++;
- new_afsi.last_played = time(NULL);
- save_afsi(&new_afsi, &afsi_obj); /* in-place update */
+ old_afsi = d->afsi;
+ d->afsi.num_played++;
+ d->afsi.last_played = time(NULL);
+ save_afsi(&d->afsi, &afsi_obj); /* in-place update */
+ d->afsi.last_played = old_afsi.last_played;
+ aced.aft_row = current_aft_row;
+ aced.old_afsi = &old_afsi;
afd.audio_format_id = d->afsi.audio_format_id;
load_chunk_table(&afd.afhi, &chunk_table_obj);
- aced.aft_row = current_aft_row;
- aced.old_afsi = &d->afsi;
/*
* No need to update the status items as the AFSI_CHANGE event will
* recreate them.