Hi,
I had similar problems and I believe the two bugs are actually one and the
same. I've solved my problems with the two patches attached.
One will change the mutexes, I fear they are done badly in the original code.
The other will fix the id3v2 tag reading, it will not find such a tag if it's
not right at the start, but I don't think we should replicate the logic and
just find some library to do this correctly.
This second patch solved my problems and is the prime suspect for the two bugs
this mail is sent to.
The problem is that we get to an endless loop and never going back to check if
we need to exit the thread so xmms will not exit, it also prevents further
submissions to AudioScrobbler.
Baruch
diff -ur xmms-scrobbler-0.3.8.1.orig/tags/id3v2.c
xmms-scrobbler-0.3.8.1/tags/id3v2.c
--- xmms-scrobbler-0.3.8.1.orig/tags/id3v2.c 2005-02-20 19:11:22.000000000
+0000
+++ xmms-scrobbler-0.3.8.1/tags/id3v2.c 2005-05-02 19:14:21.000000000 +0100
@@ -556,71 +556,28 @@
charsRead = fread(tag_buffer, 1, 10, fp);
pos = 0;
bp = tag_buffer;
- while(status == 0 && !feof(fp))
- {
- if(search == -1)
- {
- if((strncmp(bp, "ID3", 3) == 0 ||
- strncmp(bp, "3DI", 3) == 0))
- status = 1;
- else
- {
- fseek(fp, 3, SEEK_END);
- charsRead = fread(tag_buffer, 1, 3, fp);
- search = -2;
- }
- }
- else
- {
- if(search == -2)
- {
- bp = tag_buffer;
- pos = ftell(fp);
- if((strncmp(bp, "ID3", 3) == 0 ||
- strncmp(bp, "3DI", 3) == 0)) status = 1;
- search = 1;
- }
- if(status != 1)
- {
- pos = ftell(fp) - BUFFER_SIZE;
- fseek(fp, pos, SEEK_SET);
- charsRead = fread(tag_buffer, 1, BUFFER_SIZE,
- fp);
-
- bp = tag_buffer;
- for(i = 0; i < charsRead - 3 && status == 0;
- i++)
- {
- bp++;
- if((strncmp(bp, "ID3", 3) == 0 ||
- strncmp(bp, "3DI", 3) == 0))
- status = 1;
- }
- if(status == 1)
- pos += bp - tag_buffer;
- pos -= BUFFER_SIZE - 9;
- if((pos < -BUFFER_SIZE + 9 || ferror(fp)) &&
- status != 1)
- status = -1;
- }
- }
- /*
- * An ID3v2 tag can be detected with the following pattern:
- *
- * $49 44 33 yy yy xx zz zz zz zz
- *
- * Where yy is less than $FF, xx is the 'flags' byte and zz is
- * less than $80.
- */
- if(status == 1 && *(bp + 3) < 0xFF && *(bp + 4) < 0xFF &&
+
+ if((strncmp(bp, "ID3", 3) == 0 || strncmp(bp, "3DI", 3) == 0))
+ status = 1;
+ else
+ return -1;
+
+ /*
+ * An ID3v2 tag can be detected with the following pattern:
+ *
+ * $49 44 33 yy yy xx zz zz zz zz
+ *
+ * Where yy is less than $FF, xx is the 'flags' byte and zz is
+ * less than $80.
+ */
+ if(status == 1 && *(bp + 3) < 0xFF && *(bp + 4) < 0xFF &&
*(bp + 6) < 0x80 && *(bp + 7) < 0x80 &&
*(bp + 8) < 0x80 && *(bp + 9) < 0x80)
- status = 1;
- else if(status != -1)
- status = 0;
- if(search == 0)
- search = -1;
- }
+ status = 1;
+ else if(status != -1)
+ status = 0;
+ if(search == 0)
+ search = -1;
if(status < 0 || feof(fp))
return -1;
else
diff -ur xmms-scrobbler-0.3.8.1.orig/scrobbler.c
xmms-scrobbler-0.3.8.1/scrobbler.c
--- xmms-scrobbler-0.3.8.1.orig/scrobbler.c 2005-02-21 00:45:05.000000000
+0000
+++ xmms-scrobbler-0.3.8.1/scrobbler.c 2005-05-02 18:16:14.000000000 +0100
@@ -484,7 +484,7 @@
return 0;
}
-static void sc_handlequeue(pthread_mutex_t mutex)
+static void sc_handlequeue(pthread_mutex_t *mutex)
{
GString *submitentry;
int nsubmit;
@@ -494,11 +494,11 @@
{
submitentry = g_string_new("");
- pthread_mutex_lock(&mutex);
+ pthread_mutex_lock(mutex);
nsubmit = sc_generateentry(submitentry);
- pthread_mutex_unlock(&mutex);
+ pthread_mutex_unlock(mutex);
if (nsubmit > 0)
{
@@ -509,7 +509,7 @@
if(!sc_submitentry(submitentry->str))
{
- pthread_mutex_lock(&mutex);
+ pthread_mutex_lock(mutex);
#ifdef ALLOW_MULTIPLE
q_free();
@@ -523,7 +523,7 @@
*/
dump_queue();
- pthread_mutex_unlock(&mutex);
+ pthread_mutex_unlock(mutex);
sc_sb_errors = 0;
}
@@ -745,20 +745,20 @@
pdebug("scrobbler starting up", DEBUG);
}
-void sc_addentry(pthread_mutex_t mutex, metatag_t *meta, int len)
+void sc_addentry(pthread_mutex_t *mutex, metatag_t *meta, int len)
{
- pthread_mutex_lock(&mutex);
+ pthread_mutex_lock(mutex);
q_put(meta, len);
/*
* This will help make sure the queue will be saved on a nasty
* segfault...
*/
dump_queue();
- pthread_mutex_unlock(&mutex);
+ pthread_mutex_unlock(mutex);
}
/* Call periodically from the plugin */
-int sc_idle(pthread_mutex_t mutex)
+int sc_idle(pthread_mutex_t *mutex)
{
sc_checkhandshake();
if (sc_hs_status)
diff -ur xmms-scrobbler-0.3.8.1.orig/scrobbler.h
xmms-scrobbler-0.3.8.1/scrobbler.h
--- xmms-scrobbler-0.3.8.1.orig/scrobbler.h 2004-09-11 04:27:03.000000000
+0100
+++ xmms-scrobbler-0.3.8.1/scrobbler.h 2005-05-02 18:20:50.000000000 +0100
@@ -1,8 +1,8 @@
#ifndef NET_H
#define NET_H 1
-int sc_idle(pthread_mutex_t);
+int sc_idle(pthread_mutex_t *);
void sc_init(char *, char *);
-void sc_addentry(pthread_mutex_t, metatag_t *, int);
+void sc_addentry(pthread_mutex_t *, metatag_t *, int);
void sc_cleaner(void);
int sc_catch_error(void);
char *sc_fetch_error(void);
diff -ur xmms-scrobbler-0.3.8.1.orig/xmms_scrobbler.c
xmms-scrobbler-0.3.8.1/xmms_scrobbler.c
--- xmms-scrobbler-0.3.8.1.orig/xmms_scrobbler.c 2005-02-21
01:25:47.000000000 +0000
+++ xmms-scrobbler-0.3.8.1/xmms_scrobbler.c 2005-05-02 18:15:22.000000000
+0100
@@ -39,7 +39,7 @@
static void cleanup(void);
static void *xs_thread(void *);
static void *hs_thread(void *);
-static int going;
+static volatile int going;
static pthread_t pt_scrobbler;
static pthread_mutex_t m_scrobbler = PTHREAD_MUTEX_INITIALIZER;
@@ -100,10 +100,7 @@
if (!going)
return;
pdebug("about to lock mutex", DEBUG);
- pthread_mutex_lock(&m_scrobbler);
- pdebug("locked mutex", DEBUG);
going = 0;
- pthread_mutex_unlock(&m_scrobbler);
pdebug("joining threads", DEBUG);
pthread_join(pt_scrobbler, &dummy);
@@ -354,12 +351,12 @@
static void *xs_thread(void *data)
{
- int run = 1, i;
+ int i;
char *charpos, *dirname;
gboolean direxists;
submit_t dosubmit;
- while (run) {
+ while (going) {
/* Error catching */
if(sc_catch_error())
{
@@ -449,7 +446,7 @@
pdebug(fmt_vastr(
"submitting artist: %s, title: %s",
meta->artist, meta->title), DEBUG);
- sc_addentry(m_scrobbler, meta,
+ sc_addentry(&m_scrobbler, meta,
dosubmit.len/1000);
}
else
@@ -460,9 +457,6 @@
g_free(fname);
metatag_delete(meta);
}
- pthread_mutex_lock(&m_scrobbler);
- run = going;
- pthread_mutex_unlock(&m_scrobbler);
usleep(100000);
}
pdebug("scrobbler thread: exiting", DEBUG);
@@ -471,20 +465,13 @@
static void *hs_thread(void *data)
{
- int run = 1;
-
- while(run)
+ while(going)
{
- if(sc_idle(m_scrobbler))
+ if(sc_idle(&m_scrobbler))
{
pdebug("Giving up due to fatal error", DEBUG);
- pthread_mutex_lock(&m_scrobbler);
going = 0;
- pthread_mutex_lock(&m_scrobbler);
}
- pthread_mutex_lock(&m_scrobbler);
- run = going;
- pthread_mutex_unlock(&m_scrobbler);
sleep(1);
}
pdebug("handshake thread: exiting", DEBUG);
