crash when using lqueue in Play Queue view

[Javier Rojas]

Hi,

I'm getting a crash (segmentation fault) when using the :lqueue command
in the Play Queue view. This is happening in the HEAD version of cmus in
its git repository (commit d6d8a2f5919df815609f0c98229b3a291f6f6da2).

I did a 'git bisect' to locate the issue, checking from 2.3 to HEAD.
According to it, f05fa4892525c6c1540e6983d8a42e326469d76f is the first
bad commit.

I attach to this mail a tarball with a few backtraces (not all
backtraces were the same in each segfault), indicating in the backtrace
file what I did to provoke the segfault. I also include the
cmus-debug.txt file generated each time, along with each backtrace.

This is a Debian (unstable) box, amd64 arch. Dunno what other system
info. might be useful to track and solve the issue.

[Johannes Weißl]

Hello Javier,

Post by Javier Rojas
I'm getting a crash (segmentation fault) when using the :lqueue command
in the Play Queue view. This is happening in the HEAD version of cmus in
its git repository (commit d6d8a2f5919df815609f0c98229b3a291f6f6da2).
I did a 'git bisect' to locate the issue, checking from 2.3 to HEAD.
According to it, f05fa4892525c6c1540e6983d8a42e326469d76f is the first
bad commit.
I attach to this mail a tarball with a few backtraces (not all
backtraces were the same in each segfault), indicating in the backtrace
file what I did to provoke the segfault. I also include the
cmus-debug.txt file generated each time, along with each backtrace.
This is a Debian (unstable) box, amd64 arch. Dunno what other system
info. might be useful to track and solve the issue.

Thanks for your bug report and all the additional information! I can
reproduce the crash, it has a very similar cause to the bug that was fixed
in 8ec61d412b7a55fbdcadbda32a71d28114ed3e94 (fix segfault when adding to
queue). I will soon post a patch which will resolve all these play queue
crashes!

The problem is that I changed the "editable" object to use red black trees
(because it is way faster for sorted inserts), but the play queue still
uses a normal linked list (it has no sorted inserts).

When modifying the play queue using editable_* functions, the linked
list and the red black tree start to diverge...


Greetings,
Johannes

[gt]

Post by Johannes Weißl
Hello Javier,

Post by Javier Rojas
I'm getting a crash (segmentation fault) when using the :lqueue command
in the Play Queue view. This is happening in the HEAD version of cmus in
its git repository (commit d6d8a2f5919df815609f0c98229b3a291f6f6da2).
I did a 'git bisect' to locate the issue, checking from 2.3 to HEAD.
According to it, f05fa4892525c6c1540e6983d8a42e326469d76f is the first
bad commit.
I attach to this mail a tarball with a few backtraces (not all
backtraces were the same in each segfault), indicating in the backtrace
file what I did to provoke the segfault. I also include the
cmus-debug.txt file generated each time, along with each backtrace.
This is a Debian (unstable) box, amd64 arch. Dunno what other system
info. might be useful to track and solve the issue.

Thanks for your bug report and all the additional information! I can
reproduce the crash, it has a very similar cause to the bug that was fixed
in 8ec61d412b7a55fbdcadbda32a71d28114ed3e94 (fix segfault when adding to
queue). I will soon post a patch which will resolve all these play queue
crashes!
The problem is that I changed the "editable" object to use red black trees
(because it is way faster for sorted inserts), but the play queue still
uses a normal linked list (it has no sorted inserts).
When modifying the play queue using editable_* functions, the linked
list and the red black tree start to diverge...

Hey Johannes,

I didn't know of this lqueue command, but after reading this thread i
tried it.

Seems that instead of adding random albums, it adds the same album N
number of times. So you may want to take a look at it too. I am
currently using your shuffle branch.

[Johannes Weißl]

Hi gt,

Post by gt
I didn't know of this lqueue command, but after reading this thread i
tried it.
Seems that instead of adding random albums, it adds the same album N
number of times. So you may want to take a look at it too. I am
currently using your shuffle branch.

Yes, this is definitely a bug! I send a patch to the mailing list
(solution are just three characters "a->" :-). Thanks for reporting!


Greetings,
Johannes

[Johannes Weißl]

queue has to be modified using queue functions, not editable!

Reported-by: Javier Rojas <***@devnull.li>
---
command_mode.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/command_mode.c b/command_mode.c
index 5593990..379a8ef 100644
--- a/command_mode.c
+++ b/command_mode.c
@@ -1859,7 +1859,7 @@ static void cmd_lqueue(char *arg)
struct rb_node *tmp;

rb_for_each_entry(t, tmp, &album->track_root, tree_node)
- editable_add(&pq_editable, simple_track_new(tree_track_info(t)));
+ play_queue_append(tree_track_info(t));
free(a);
item = next;
} while (item != &head);
@@ -1867,6 +1867,11 @@ unlock:
editable_unlock();
}

+struct track_list {
+ struct list_head node;
+ const struct simple_track *track;
+};
+
static void cmd_tqueue(char *arg)
{
LIST_HEAD(head);
@@ -1893,13 +1898,14 @@ static void cmd_tqueue(char *arg)
item = lib_editable.head.next;
pos = 0;
for (i = 0; i < count; i++) {
- struct simple_track *t;
+ struct track_list *t;

while (pos < r[i]) {
item = item->next;
pos++;
}
- t = simple_track_new(to_simple_track(item)->info);
+ t = xnew(struct track_list, 1);
+ t->track = to_simple_track(item);
list_add_rand(&head, &t->node, i);
}
free(r);
@@ -1907,8 +1913,9 @@ static void cmd_tqueue(char *arg)
item = head.next;
do {
struct list_head *next = item->next;
- struct simple_track *t = to_simple_track(item);
- editable_add(&pq_editable, t);
+ struct track_list *t = container_of(item, struct track_list, node);
+ play_queue_append(t->track->info);
+ free(t);
item = next;
} while (item != &head);
unlock:

[Johannes Weißl]

lqueue N did append the same album N times!

Reported-by: gt <***@gmail.com>
---
command_mode.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/command_mode.c b/command_mode.c
index 379a8ef..3905fd9 100644
--- a/command_mode.c
+++ b/command_mode.c
@@ -1858,7 +1858,7 @@ static void cmd_lqueue(char *arg)
struct tree_track *t;
struct rb_node *tmp;

- rb_for_each_entry(t, tmp, &album->track_root, tree_node)
+ rb_for_each_entry(t, tmp, &a->album->track_root, tree_node)
play_queue_append(tree_track_info(t));
free(a);
item = next;

[Javier Rojas]

Thanks, I applied the patch and so far I've had no crashes at all.

[Gregory Petrosyan]

Post by Javier Rojas
Thanks, I applied the patch and so far I've had no crashes at all.

Thanks for reporting and testing, the patches are now in the git (both in
master and maint branches).

Gregory