devs/foxbox
1
0
Fork 0
forked from len0rd/rockbox
Code Pull requests Activity

playlist_create fix race condition

Browse source 
I'm pretty sure this is a very old bug I traced it down to the
current_playlist getting changed out from under add_indices_to_playlist
causing myriad of issues from buffer full to invalid control file to shifting indices

this only appears to happen with the dircache on

I still get an incorrect resume state with the wrong song very rarely -- turns out get_filename seeks the file FIXED

Some debugging left in for now till we can verify there are no other instances

Change-Id: I289a775462eddfe93da4a326dc9e38605af06816
This commit is contained in:
William Wilgus 2022-11-29 01:30:47 -05:00
parent 0ba3392b9f
commit c9c340704f
1 changed files with 26 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

32
apps/playlist.c
View file

@ -562,6 +562,7 @@ static void update_playlist_filename(struct playlist_info* playlist,
static int add_indices_to_playlist(struct playlist_info* playlist, static int add_indices_to_playlist(struct playlist_info* playlist,
char* buffer, size_t buflen) char* buffer, size_t buflen)
{ {
int playlist_fd;
ssize_t nread; ssize_t nread;
unsigned int i = 0; unsigned int i = 0;
unsigned int count = 0; unsigned int count = 0;
@ -572,10 +573,14 @@ static int add_indices_to_playlist(struct playlist_info* playlist,
if (!buflen) if (!buflen)
buffer = alloca((buflen = 64)); buffer = alloca((buflen = 64));
mutex_lock(playlist->control_mutex); /* read can yield! */
if(-1 == playlist->fd) if(-1 == playlist->fd)
playlist->fd = open_utf8(playlist->filename, O_RDONLY); playlist->fd = open_utf8(playlist->filename, O_RDONLY);
if(playlist->fd < 0) if(playlist->fd < 0)
return -1; /* failure */ return -1; /* failure */
playlist_fd = playlist->fd;
if((i = lseek(playlist->fd, 0, SEEK_CUR)) > 0) if((i = lseek(playlist->fd, 0, SEEK_CUR)) > 0)
playlist->utf8 = true; /* Override any earlier indication. */ playlist->utf8 = true; /* Override any earlier indication. */
@ -583,9 +588,17 @@ static int add_indices_to_playlist(struct playlist_info* playlist,
store_index = true; store_index = true;
playlist->fd = -2; /* DEBUGGING Remove me! */
/* invalidate playlist in case we yield */
int amount = playlist->amount;
playlist->amount = -1;
int max_playlist_size = playlist->max_playlist_size;
playlist->max_playlist_size = 0;
while(1) while(1)
{ {
nread = read(playlist->fd, buffer, buflen); nread = read(playlist_fd, buffer, buflen);
/* Terminate on EOF */ /* Terminate on EOF */
if(nread <= 0) if(nread <= 0)
break; break;
@ -605,18 +618,18 @@ static int add_indices_to_playlist(struct playlist_info* playlist,
if(*p != '#') if(*p != '#')
{ {
if ( playlist->amount >= playlist->max_playlist_size ) { if ( amount >= max_playlist_size ) {
display_buffer_full(); display_buffer_full();
result = -1; result = -1;
goto exit; goto exit;
} }
/* Store a new entry */ /* Store a new entry */
playlist->indices[ playlist->amount ] = i+count; playlist->indices[ amount ] = i+count;
#ifdef HAVE_DIRCACHE #ifdef HAVE_DIRCACHE
copy_filerefs(&playlist->dcfrefs[playlist->amount], NULL, 1); copy_filerefs(&playlist->dcfrefs[amount], NULL, 1);
#endif #endif
playlist->amount++; amount++;
} }
} }
} }
@ -625,6 +638,14 @@ static int add_indices_to_playlist(struct playlist_info* playlist,
} }
exit: exit:
/* v DEBUGGING Remove me! */
if (playlist->fd != -2)
splashf(HZ, "Error playlist fd");
playlist->fd = playlist_fd;
/* ^ DEBUGGING Remove me! */
playlist->amount = amount;
playlist->max_playlist_size = max_playlist_size;
mutex_unlock(playlist->control_mutex);
#ifdef HAVE_DIRCACHE #ifdef HAVE_DIRCACHE
queue_post(&playlist_queue, PLAYLIST_LOAD_POINTERS, 0); queue_post(&playlist_queue, PLAYLIST_LOAD_POINTERS, 0);
#endif #endif
@ -2122,7 +2143,6 @@ int playlist_create(const char *dir, const char *file)
void* buf = core_get_data(handle); void* buf = core_get_data(handle);
STORAGE_ALIGN_BUFFER(buf, buflen); STORAGE_ALIGN_BUFFER(buf, buflen);
buflen = ALIGN_DOWN(buflen, 512); /* to avoid partial sector I/O */ buflen = ALIGN_DOWN(buflen, 512); /* to avoid partial sector I/O */
/* load the playlist file */ /* load the playlist file */
add_indices_to_playlist(playlist, buf, buflen); add_indices_to_playlist(playlist, buf, buflen);
core_free(handle); core_free(handle);