Bug#998310: [Pkg-mpd-maintainers] Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-06 Thread kaliko

On 11/6/21 10:39 AM, Florian Schlichting wrote:

On Fri, Nov 05, 2021 at 11:52:14AM +0100, kaliko wrote:

Then I believe setting NDEBUG (either at meson level with "b_ndebug=true" or
within "DEB_CXXFLAGS_MAINT_APPEND") will lead to the same result as meson
options "--buildtype=debugoptimized -Db_ndebug=true" since dpkg-buildflags
already set "-g -O2".

What do you think of this Florian?


I think that sounds sensible. 
[…]

Will you make the change?


Pushed to master on salsa. I did not update the changelog.

k.



OpenPGP_signature
Description: OpenPGP digital signature


Bug#998310: [Pkg-mpd-maintainers] Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-06 Thread Florian Schlichting
Hi kaliko,

On Fri, Nov 05, 2021 at 11:52:14AM +0100, kaliko wrote:
> Then I believe setting NDEBUG (either at meson level with "b_ndebug=true" or
> within "DEB_CXXFLAGS_MAINT_APPEND") will lead to the same result as meson
> options "--buildtype=debugoptimized -Db_ndebug=true" since dpkg-buildflags
> already set "-g -O2".
> 
> What do you think of this Florian?

I think that sounds sensible. We want to generate debug info for
dbgsym/gdb, and as Max says he intends the assertions for hunting down
bugs and doesn't expect normal users to run builds that include them,
disabling them is the correct thing to do.

Following your links I found Ian's explanation at
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=711515#30, which
probably explains the thinking behind Debian's defaults, and why this is
a situation where we should deviate from them.

Will you make the change?

Florian



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-05 Thread kaliko

On 11/4/21 8:38 PM, Max Kellermann wrote:

On 2021/11/04 19:32, kaliko  wrote:

I had a look at the 0.23.3-1 build and actually debhelper calls meson with
"--wrap-mode=nodownload --buildtype=plain --prefix=/usr …" plus some *dir
options [2]. Then indeed asserts are not disabled but it's not really a
debug build either in meson terminology at least [0].


It's a debug build if debug options are enabled.  There's are two generic
Meson debug options […]


Thanks for your answer Max :)

Then I believe setting NDEBUG (either at meson level with 
"b_ndebug=true" or within "DEB_CXXFLAGS_MAINT_APPEND") will lead to the 
same result as meson options "--buildtype=debugoptimized 
-Db_ndebug=true" since dpkg-buildflags already set "-g -O2".


What do you think of this Florian?


I am MPD upstream, and I do not just "consider" that, but it's an
undisputable fact.


Sorry for my wording Max, I did not meant to question or doubt your 
expertise. I'm made a build and regarding runtime exe size the gain is 
significant: ~1.8MB → ~1.6MB.



[…]
IMHO all Debian packages should build with NDEBUG, so this should
really be a central debhelper feature.


This has been raised before:

   https://lists.debian.org/debian-devel/2013/02/msg00124.html
   https://lists.debian.org/debian-mentors/2018/04/msg00244.html

It's not even in meson/ninja debhelper implementation (as I thought) but 
actually in global build options exposed by dpkg.

Why dpkg-buildflags does not expose "-DNDEBUG"?
I believe there are no consensus in Debian community (cf. 2013 mail 
above) or, surprisingly, this has yet to be addressed in the project.


But that's beyond the scope of this report and MPD packaging…

k.



OpenPGP_signature
Description: OpenPGP digital signature


Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-04 Thread Max Kellermann
On 2021/11/04 19:32, kaliko  wrote:
> I had a look at the 0.23.3-1 build and actually debhelper calls meson with
> "--wrap-mode=nodownload --buildtype=plain --prefix=/usr …" plus some *dir
> options [2]. Then indeed asserts are not disabled but it's not really a
> debug build either in meson terminology at least [0].

It's a debug build if debug options are enabled.  There's are two generic
Meson debug options:

- "b_ndebug" (negated: which *disables* debugging features such as
  runtime assertions) - disabled by default, i.e. assertions are
  *enabled*

- "cpp_debugstl" is essentially the same, just for the C++ standard
  library, and it's not negated.  This one is disabled by default,
  i.e. assertions are *disabled*

Note that "--buildtype=debug" does NOT enable a debug build.  The name
is really confusing.  Meson documentation says:

 "debug info is generated but the result is not optimized"

This is about debug info (GCC option "-g"), which is of course needed
to debug a program in gdb, but it's not at all about enabling or
disabling debugging features in the generated code.  Debug info has no
effect on the code or on the performance.

(btw. "--buildtype=release" is very much useless.
"--buildtype=debugoptimized" generates the same code, but doesn't omit
debug info, which can be useful after a crash has occurred.  Without
debug info, crash dumps cannot be analyzed, and debug info cannot ever
be reproduced if it has been omitted in the build.)

> Anyway the way we build the package differs from upstream recommendations
> [1]: " --buildtype=debugoptimized -Db_ndebug=true".
> 
> Correct me if I'm wrong Max, but upstream considers this has an impact on
> runtime performances.

I am MPD upstream, and I do not just "consider" that, but it's an
undisputable fact.  And it is a fact for *all* C or C++ programs which
use assert().  Which is a very common thing.  Not defining NDEBUG has
a considerable effect on executable size, compiler optimizations and
runtime performance.  This overhead is useful when searching for bugs,
but not useful for normal users of an application.

IMHO all Debian packages should build with NDEBUG, so this should
really be a central debhelper feature.

> The question is then, should we add "b_ndebug=true"?
> I did not find any mention of NBDEBUG in policies.

Indeed Debian does not mention this anywhere.  But as I said, it
should be added to the policy (and to debhelper).

Max



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-04 Thread kaliko

Hi,

03/11/2021 18:34, Max Kellermann wrote:

On 2021/11/03 18:00, Benjamin Francois  wrote:

Confirmed, I commented out the pid_file line in /etc/mpd.conf and mpd now 
starts properly. Thanks Sir!


Okay, this is now fixed upstream:

  
https://github.com/MusicPlayerDaemon/MPD/commit/14b3c0f0afe691739cdfc71d71adf740114d5a98

This problem affected only debug builds; this was a false-positive
assertion failure, and my fix just works around this by reordering the
destructor calls.

I was surprised to learn that all Debian packages appear to have
debugging enabled, which adds a lot of unnecessary overhead.


I had a look at the 0.23.3-1 build and actually debhelper calls meson 
with "--wrap-mode=nodownload --buildtype=plain --prefix=/usr …" plus 
some *dir options [2]. Then indeed asserts are not disabled but it's not 
really a debug build either in meson terminology at least [0].


Anyway the way we build the package differs from upstream 
recommendations [1]: " --buildtype=debugoptimized -Db_ndebug=true".


Correct me if I'm wrong Max, but upstream considers this has an impact 
on runtime performances.


The question is then, should we add "b_ndebug=true"?
I did not find any mention of NBDEBUG in policies.

k.


[0] https://mesonbuild.com/Builtin-options.html#base-options
[1] https://mpd.readthedocs.io/en/stable/user.html#compiling-from-source
[2] cf. line 2228: 
https://buildd.debian.org/status/fetch.php?pkg=mpd=amd64=0.23.3-1=1635781704=0




OpenPGP_signature
Description: OpenPGP digital signature


Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-03 Thread Max Kellermann
On 2021/11/03 18:00, Benjamin Francois  wrote:
> Confirmed, I commented out the pid_file line in /etc/mpd.conf and mpd now 
> starts properly. Thanks Sir!

Okay, this is now fixed upstream:

 
https://github.com/MusicPlayerDaemon/MPD/commit/14b3c0f0afe691739cdfc71d71adf740114d5a98

This problem affected only debug builds; this was a false-positive
assertion failure, and my fix just works around this by reordering the
destructor calls.

I was surprised to learn that all Debian packages appear to have
debugging enabled, which adds a lot of unnecessary overhead.



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-03 Thread Benjamin Francois
Confirmed, I commented out the pid_file line in /etc/mpd.conf and mpd now 
starts properly. Thanks Sir!

On Nov 3 2021, at 9:42 am, Max Kellermann  wrote:
> On 2021/11/03 17:36, Max Kellermann  wrote:
> > You configured a pid_file which MPD was unable to write; maybe because
> > it failed file permissions, or maybe because the containing directory
> > does not exist.
>
> btw. if you use systemd, there's no point in configuring a pid_file.
> PID files are an obsolete concept which MPD supports only for legacy
> reasons.
>



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-03 Thread Max Kellermann
On 2021/11/03 17:36, Max Kellermann  wrote:
> You configured a pid_file which MPD was unable to write; maybe because
> it failed file permissions, or maybe because the containing directory
> does not exist.

btw. if you use systemd, there's no point in configuring a pid_file.
PID files are an obsolete concept which MPD supports only for legacy
reasons.



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-03 Thread Max Kellermann
On 2021/11/03 17:23, Benjamin Francois  wrote:
> Catchpoint 1 (exception thrown), 0x736a5322 in __cxa_throw () from 
> /lib/x86_64-linux-gnu/libstdc++.so.6
> (gdb) bt
> #0 0x736a5322 in __cxa_throw () at 
> /lib/x86_64-linux-gnu/libstdc++.so.6
> #1 0x5558ba6e in PidFile::PidFile(AllocatedPath const&) 
> (path=, this=) at ../src/unix/PidFile.hxx:46
> #2 daemonize_commit() () at ../src/unix/Daemon.cxx:212

You configured a pid_file which MPD was unable to write; maybe because
it failed file permissions, or maybe because the containing directory
does not exist.

This is a configuration error which triggers the crash bug.  If this
crash bug wouldn't exist, MPD still wouldn't be able to start due to
that misconfiguration, but at least it would log a helpful error
message.



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-03 Thread Benjamin Francois
On Wed%2C 3 Nov 2021 07%3A08%3A23 %2B0100 Max Kellermann wrote%3A
> Thanks%2C that was almost helpful - but you did not install mpd-dbgsym%2C

Whoops :'D let's try one more time then.
❯ sudo gdb --args mpd --stderr --no-daemon --verbose
GNU gdb (Debian 10.1-2) 10.1.90.20210103-git
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from mpd...
Reading symbols from 
/usr/lib/debug/.build-id/26/be37e73d656eddadb9ab98d3becf1abd7d1696.debug...
(gdb) catch throw
Catchpoint 1 (throw)
(gdb) run
Starting program: /usr/bin/mpd --stderr --no-daemon --verbose
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
config_file: loading file /etc/mpd.conf
path: SetFSCharset: fs charset is
libsamplerate: libsamplerate converter 'Fastest Sinc Interpolator'
vorbis: Xiph.Org libVorbis 1.3.7
opus: libopus 1.3.1
sndfile: libsndfile-1.0.31
adplug: adplug 2.3.3
simple_db: reading DB

Catchpoint 1 (exception thrown), 0x736a5322 in __cxa_throw () from 
/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) bt
#0 0x736a5322 in __cxa_throw () at /lib/x86_64-linux-gnu/libstdc++.so.6
#1 0x5559e71f in db_load_internal(TextFile&, Directory&) 
(file=, music_root=) at 
../src/db/plugins/simple/DatabaseSave.cxx:112
#2 0x5565fa6c in SimpleDatabase::Load() (this=0x5578a040) at 
../src/db/plugins/simple/SimpleDatabasePlugin.cxx:158
#3 0x55660721 in SimpleDatabase::Open() (this=0x5578a040) at 
../src/db/plugins/simple/SimpleDatabasePlugin.cxx:178
#4 0x555a4aed in glue_db_init_and_load (config=..., instance=...) at 
../src/Main.cxx:221
#5 InitDatabaseAndStorage (config=..., instance=...) at ../src/Main.cxx:244
#6 MainConfigured(options const&, ConfigData const&) (options=, 
raw_config=...) at ../src/Main.cxx:444
#7 0x555a4d4d in MainOrThrow(int, char**) (argc=, 
argv=) at ../src/Main.cxx:650
#8 0x555a302a in mpd_main(int, char**) (argv=, 
argc=) at ../src/Main.cxx:656
#9 main(int, char**) (argc=, argv=) at 
../src/Main.cxx:668
(gdb) cont
Continuing.
exception: Tag list mismatch, discarding database file
curl: version 7.74.0
curl: with GnuTLS/3.7.2

Catchpoint 1 (exception thrown), 0x736a5322 in __cxa_throw () from 
/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) bt
#0 0x736a5322 in __cxa_throw () at /lib/x86_64-linux-gnu/libstdc++.so.6
#1 0x5558ba6e in PidFile::PidFile(AllocatedPath const&) 
(path=, this=) at ../src/unix/PidFile.hxx:46
#2 daemonize_commit() () at ../src/unix/Daemon.cxx:212
#3 0x555a44ed in MainConfigured(options const&, ConfigData const&) 
(options=, raw_config=...) at ../src/Main.cxx:468
#4 0x555a4d4d in MainOrThrow(int, char**) (argc=, 
argv=) at ../src/Main.cxx:650
#5 0x555a302a in mpd_main(int, char**) (argv=, 
argc=) at ../src/Main.cxx:656
#6 main(int, char**) (argc=, argv=) at 
../src/Main.cxx:668
(gdb) cont
Continuing.
mpd: ../src/event/Loop.cxx:60: EventLoop::~EventLoop(): Assertion 
`sockets.empty()' failed.

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
49 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
#1 0x732db536 in __GI_abort () at abort.c:79
#2 0x732db41f in __assert_fail_base
(fmt=0x734426b0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
assertion=0x5569afa2 "sockets.empty()", file=0x5569af5c 
"../src/event/Loop.cxx", line=60, function=) at assert.c:92
#3 0x732ea7f2 in __GI___assert_fail
(assertion=0x5569afa2 "sockets.empty()", file=0x5569af5c 
"../src/event/Loop.cxx", line=60, function=0x5569af44 
"EventLoop::~EventLoop()")
at assert.c:101
#4 0x555e5458 in EventLoop::~EventLoop() 
(this=this@entry=0x7fffcbc8, __in_chrg=) at 
../src/event/Loop.cxx:60
#5 0x555b7c56 in EventThread::~EventThread() (this=0x7fffcbc8, 
__in_chrg=) at ../src/event/Thread.hxx:43
#6 Instance::~Instance() (this=this@entry=0x7fffc310, __in_chrg=) at ../src/Instance.cxx:75
#7 0x555867a1 in MainConfigured(options const&, ConfigData const&) 
(options=, raw_config=) at ../src/Main.cxx:578
#8 0x555a4d4d in MainOrThrow(int, char**) (argc=, 
argv=) at ../src/Main.cxx:650
#9 

Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-03 Thread Max Kellermann
On 2021/11/02 23:47, Benjamin Francois  wrote:
> On Tue, 2 Nov 2021 13:53:20 +0100 Max Kellermann wrote:
> > TLDR: it's complicated, and we can't see the real error just yet.
> 
> Hello! I am having the same issue so I thought I'd give it a go.

Thanks, that was almost helpful - but you did not install mpd-dbgsym,
that's why I see only:

> #1 0x5558ba6e in ()
> #2 0x555a44ed in ()
> #3 0x555a4d4d in ()
> #4 0x555a302a in ()

Can you try again with mpd-dbgsym, please?



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-02 Thread Benjamin Francois
On Tue, 2 Nov 2021 13:53:20 +0100 Max Kellermann wrote:
> TLDR: it's complicated, and we can't see the real error just yet.

Hello! I am having the same issue so I thought I'd give it a go.
❯ sudo gdb --args mpd --stderr --no-daemon --verbose
GNU gdb (Debian 10.1-2) 10.1.90.20210103-git
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from mpd...
(No debugging symbols found in mpd)
(gdb) catch throw
Catchpoint 1 (throw)
(gdb) run
Starting program: /usr/bin/mpd --stderr --no-daemon --verbose
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
config_file: loading file /etc/mpd.conf
path: SetFSCharset: fs charset is
libsamplerate: libsamplerate converter 'Fastest Sinc Interpolator'
vorbis: Xiph.Org libVorbis 1.3.7
opus: libopus 1.3.1
sndfile: libsndfile-1.0.31
adplug: adplug 2.3.3
simple_db: reading DB

Catchpoint 1 (exception thrown), 0x736a6322 in __cxa_throw () from 
/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) bt
#0 0x736a6322 in __cxa_throw () at /lib/x86_64-linux-gnu/libstdc++.so.6
#1 0x5559e71f in ()
#2 0x5565fa6c in ()
#3 0x55660721 in ()
#4 0x555a4aed in ()
#5 0x555a4d4d in ()
#6 0x555a302a in ()
#7 0x732dde4a in __libc_start_main (main=
0x555a3010, argc=4, argv=0x7fffe5f8, init=, 
fini=, rtld_fini=, stack_end=0x7fffe5e8) at 
../csu/libc-start.c:314
#8 0x555a33da in ()
(gdb) cont
Continuing.
exception: Tag list mismatch, discarding database file
curl: version 7.74.0
curl: with GnuTLS/3.7.2

Catchpoint 1 (exception thrown), 0x736a6322 in __cxa_throw () from 
/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) bt
#0 0x736a6322 in __cxa_throw () at /lib/x86_64-linux-gnu/libstdc++.so.6
#1 0x5558ba6e in ()
#2 0x555a44ed in ()
#3 0x555a4d4d in ()
#4 0x555a302a in ()
#5 0x732dde4a in __libc_start_main (main=
0x555a3010, argc=4, argv=0x7fffe5f8, init=, 
fini=, rtld_fini=, stack_end=0x7fffe5e8) at 
../csu/libc-start.c:314
#6 0x555a33da in ()
(gdb) cont
Continuing.
mpd: ../src/event/Loop.cxx:60: EventLoop::~EventLoop(): Assertion 
`sockets.empty()' failed.

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
49 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
#1 0x732dc536 in __GI_abort () at abort.c:79
#2 0x732dc41f in __assert_fail_base
(fmt=0x734436b0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
assertion=0x5569afa2 "sockets.empty()", file=0x5569af5c 
"../src/event/Loop.cxx", line=60, function=) at assert.c:92
#3 0x732eb7f2 in __GI___assert_fail
(assertion=0x5569afa2 "sockets.empty()", file=0x5569af5c 
"../src/event/Loop.cxx", line=60, function=0x5569af44 
"EventLoop::~EventLoop()") at assert.c:101
#4 0x555e5458 in ()
#5 0x555b7c56 in ()
#6 0x555867a1 in ()
#7 0x555a4d4d in ()
#8 0x555a302a in ()
#9 0x732dde4a in __libc_start_main (main=
0x555a3010, argc=4, argv=0x7fffe5f8, init=, 
fini=, rtld_fini=, stack_end=0x7fffe5e8) at 
../csu/libc-start.c:314
#10 0x555a33da in ()
(gdb) cont
Continuing.

Program terminated with signal SIGABRT, Aborted.
The program no longer exists.
(gdb)

Hope this helps!
Cheers,
Ben.



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-02 Thread Max Kellermann
On 2021/11/02 13:19, Antoine Le Gonidec  wrote:
> Here is what I get when trying to start mpd from gdb, following the 
> instructions you linked:

Thanks, this and your backtrace shows that MPD startup has failed for
some reason, and a C++ exception got thrown with details about the
error, but before the exception gets caught and logged, MPD shuts
itself down, which is where this crash occurs because there's some bug
in the shutdown procedure (a bug which only occurs if shutting down
due to startup error).

TLDR: it's complicated, and we can't see the real error just yet.

Since you appear to have some gdb skills, hooray, please do the
following: run MPD in gdb, type "catch throw", then "run", and when
gdb stops execution due to an exception, get that backtrace using
"bt".  Try "cont" and "bt" again until it crashes; if there are more
backtraces, copy them as well.  I hope that gives me a hint what
exactly is failing, and how to get a grip on the crash problem.



Bug#998310: mpd: fails to start with "Assertion `sockets.empty()' failed."

2021-11-02 Thread Max Kellermann
On 2021/11/02 09:43, Antoine Le Gonidec  wrote:
> Package: mpd
> Version: 0.23.3-1
> Severity: important
> 
> Since the mpd upgrade from 0.22.10-1+b1 to 0.23.3-1, it fails to start
> with the following error:
> 
> # /usr/bin/mpd --no-daemon
> Nov 02 09:38 : exception: Tag list mismatch, discarding database file
> mpd: ../src/event/Loop.cxx:60: EventLoop::~EventLoop(): Assertion
> `sockets.empty()' failed.
> Aborted

The crash message indicates that this happens during shutdown, not
during startup.  I assume you did not paste the full log; the rest
went to the Journal; to check your crash, I need the full (verbose)
log.  Please see
https://mpd.readthedocs.io/en/stable/user.html#mpd-crashes for how to
best report MPD crash bug.