Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dbus-daemon is not killed after playback complete #59

Closed
Aiexis opened this issue Sep 17, 2013 · 39 comments
Closed

dbus-daemon is not killed after playback complete #59

Aiexis opened this issue Sep 17, 2013 · 39 comments

Comments

@Aiexis
Copy link

Aiexis commented Sep 17, 2013

Everything is in the title ;)
$ ps xaf
22126 ? Ss 0:00 /usr/bin/dbus-daemon --fork --print-pid 5 --print-address 7 --session
22192 ? Ss 0:00 /usr/bin/dbus-daemon --fork --print-pid 5 --print-address 7 --session

etc...

In my case, omxplayer plays lots of file per day, and Pi hangs after one or two days, due to full memory use.

@fcarlier
Copy link

identical for me ;)

Aiexis> Can you explain me how you play with omxplayer and dbus, because now, i don't have keyboard ;( only CTRL+C

@Aiexis
Copy link
Author

Aiexis commented Sep 17, 2013

I haven't done anything special, just install omxplayer and dbus-x11 (apt-get install dbus-x11). I do not use X11 neither.

@fcarlier
Copy link

Hello,

I have a little solution, Explain :

in omxplayer, we have :

if test -z "$DBUS_SESSION_BUS_ADDRESS"; then
eval $(dbus-launch --sh-syntax)
fi

in DBus website (http://www.linuxfromscratch.org/blfs/view/svn/general/dbus.html),
the 'Boot Script' paragraphe said 'This method will not stop the session daemon when you exit your shell'

I decided to re-crosscompile 'DBus 1.6.8' with little modification :
in file ./dbus-1.6.8/tools/dbus-launch.c, line 424 :

printf ("DBUS_SESSION_BUS_ADDRESS='%s';\n", bus_address);
printf ("export DBUS_SESSION_BUS_ADDRESS;\n");
printf ("DBUS_SESSION_BUS_PID=%ld;\n", (long) bus_pid);

i add this line after :

printf ("export DBUS_SESSION_BUS_PID;\n");

And replace /usr/bin/dbus-launch with the new version

And, in omxplayer.cpp, i add :

int iDBUS_SESSION_BUS_PID = 0;

in main function :

int main(int argc, char *argv[])
{

char* pDBUS_SESSION_BUS_PID;
pDBUS_SESSION_BUS_PID = getenv("DBUS_SESSION_BUS_PID");
if (pDBUS_SESSION_BUS_PID != NULL) {
iDBUS_SESSION_BUS_PID = atoi(pDBUS_SESSION_BUS_PID);
printf("The current PID of DBus session: %s\n", pDBUS_SESSION_BUS_PID);
}

After, m_keyboard.Close();

kill(iDBUS_SESSION_BUS_PID, SIGQUIT);

Voili Voila, it's OK !!
I think that propose 'printf ("export DBUS_SESSION_BUS_PID;\n");' to developer of DBus is possible ;)

A+
Florent

@popcornmix
Copy link
Owner

@skgsergio
It seems the problem is in /usr/bin/omxplayer:

if [ -z "$DBUS_SESSION_BUS_ADDRESS" ]; then
    eval $(dbus-launch --sh-syntax)

It looks like that is intended to only run dbus-launch once, but it doesn't do that. It launches every time.
Presumably because DBUS_SESSION_BUS_ADDRESS is only defined within the environment of this script, and the next launch can't see the previous one.

@fcarlier
Copy link

after send a mail to Simon McVittie (DBus Developper)

"""
Le 24/09/13 12:38, Simon McVittie a écrit :

On 24/09/13 09:38, Florent Carlier wrote:

With dbus-1.6.8 (identical 1.6.14), we don't have PID to kill process
after quit omxplayer :
#59

If you want to export the DBUS_SESSION_BUS_PID from dbus-launch into a
child shell script's environment, you can do the "export
DBUS_SESSION_BUS_PID" yourself; but that's probably a bad idea (see below).

I believe the intention of not exporting that variable was that killing
the bus from the shell script that started it is appropriate, whereas
killing the bus from random child processes is not. The method you're
using in #59 means that if omxplayer inherits a session bus from its
caller, it will kill that "larger" session on exit - I don't think
that's really what you want :-)

Here's one way you could avoid that bug, which would break if I made the
change you suggest:

if test -z "$DBUS_SESSION_BUS_ADDRESS"; then
eval $(dbus-launch --sh-syntax)
fi

... run the C++ part of omxplayer ...

if test -z "$DBUS_SESSION_BUS_PID"; then
kill "$DBUS_SESSION_BUS_PID"
fi

Another way to do this would be to avoid dbus-launch altogether, and use
dbus-daemon's --print-pid and --print-address options to implement
something similar to, but simpler than, dbus-launch. That would also
mean you can avoid having an X11 dependency, if that matters.
http://cgit.freedesktop.org/telepathy/telepathy-glib/tree/tools/with-session-bus.sh
is one example of using those options.

dbus 1.7.x adds dbus-run-session, which is basically that shell script,
but done in C without using temporary files. In the long term, I hope to
deprecate dbus-launch - it does too many things, and none of them very well.

Regards,
S

"""

I'm testing a new call script with dbus-daemon ('deprecate dbus-launch')

Florent

@popcornmix
Copy link
Owner

Removing the X11 dependency sounds good.
@skgsergio does using dbus-dameon directly (and killing it afterwards) seem possible?

@fcarlier
Copy link

SOLUTION 👍

"""
exec 5> omxplayer.address
exec 6> omxplayer.pid

cleanup ()
{
pid=head -n1 omxplayer.pid
if test -n "$pid" ; then
kill -INT "$pid"
fi
rm -f omxplayer.address
rm -f omxplayer.pid
}

trap cleanup INT HUP TERM

dbus-daemon --fork --print-address 5 --print-pid 6 --session

DBUS_SESSION_BUS_ADDRESS="cat omxplayer.address"
export DBUS_SESSION_BUS_ADDRESS
DBUS_SESSION_BUS_PID="cat omxplayer.pid"
export DBUS_SESSION_BUS_PID

$OMXPLAYER --font $FONT --italic-font $ITALIC_FONT "$@"

trap - INT HUP TERM
cleanup
"""

Warning `cat

testing !!
I can send @popcornmix and @skgsergio the omxplayer file

@fcarlier
Copy link

If you commit the solution, I sent an email to thank Simon McVittie

@skgsergio
Copy link

I'll dig about this later, I can't atm. Maybe tomorrow moring I should be able to do it.

Anyway seems fair the change.

@fcarlier
Copy link

i propose commit

@skgsergio
Copy link

I've been so busy these days, too much work...

I've compiled yesterday a new version and I've uploaded it to http://omxplayer.sconde.net/ with @fcarlier fix adapted to my script (I'm not using the same launch script that there is in the repo).

You can see my script here: https://gist.github.com/skgsergio/6825354

I've tested it and seems that it's working without problems (Ctrl+C, q, end of the video, ...).

@andyjagoe
Copy link

Hi All, thanks for looking into this issue. Has this fix worked for others? I loop omxplayer via a python subprocess and am still having the problem of dbus-daemon not being killed using the Dec 16 commit. After a day's running, I have hundreds and hundreds of orphan dbus-daemon processes until the Raspberry Pi finally freezes up.

@Aiexis
Copy link
Author

Aiexis commented Dec 20, 2013

I still have this issue with this version. I use an old one.

@andyjagoe
Copy link

Thanks Aiexis. Very good to know. This is a pretty serious issue. Unfortunately, correct video rotation was added after dbus support...so it doesn't seem possible to have one without the other. Is it possible (or would it make sense) to disable dbus via command line option? Or maybe a command line option should be required to enable it--since only a very sophisticated user or programmer would be using dbus.

@andyjagoe
Copy link

Aiexis...I've worked around this issue by commenting the DBUS calls in the omxplayer startup script. If you're not using DBUS, this will allow you to use a more recent version of omxplayer (e.g. with correct rotation) without having an issue with dbus-daemon not getting killed. I've found the 12/16 build b34143c works best for me...I've had some aspect ratio and visibility issues with more recent version of the script.

#OMXPLAYER_DBUS_ADDR=`mktemp -t omxplayer-XXXXX`
#OMXPLAYER_DBUS_PID=`mktemp -t omxplayer-XXXXX`

#exec 5> $OMXPLAYER_DBUS_ADDR
#exec 6> $OMXPLAYER_DBUS_PID

#dbus-daemon --fork --print-address 5 --print-pid 6 --session

#DBUS_SESSION_BUS_ADDRESS=`cat $OMXPLAYER_DBUS_ADDR`
#DBUS_SESSION_BUS_PID=`cat $OMXPLAYER_DBUS_PID`

#export DBUS_SESSION_BUS_ADDRESS
#export DBUS_SESSION_BUS_PID

LD_LIBRARY_PATH="$OMXPLAYER_LIBS${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" $OMXPLAYER_BIN "$@"; true

#if [ -n "$DBUS_SESSION_BUS_PID" ]; then
#    kill -2 "$DBUS_SESSION_BUS_PID"
#fi

#rm -f $OMXPLAYER_DBUS_ADDR
#rm -f $OMXPLAYER_DBUS_PID

@KenT2
Copy link

KenT2 commented Jan 10, 2014

Just wondering if there is has any relationship with Issue #124. In the HANG1 situation omxplayer.bin does not terminate and hence I assume the code in omxplayer that comes after it does not get executed, hence the dbus daemons do not get deleted. I have seen the number of daemons increase during the 40 hour run.

@jehutting
Copy link

I don't use the omxplayer script at all to run omxplayer. I just run LD_LIBRARY_PATHxxx omxplayer.bin xxxx.

I don't see multiple dbus-daemon after multiple starts and stops of omxplayer. I use 'ps xaf | grep -i dbus', and only see ONE dbus-daemon still hanging around. It is also the dbus-daemon omxplayer has started up.

When I run the script I get

Failed to start message bus: Failed to bind socket "*:55557": Address already in use

Maybe a silly question here but what should I do TO GET multiple daemons?

@andyjagoe you commented the D-BUS stuff in your script to avoid the dbus-daemon not being killed. I assume without the dbus-daemon forking your application (on top of omxplayer) runs just fine. Do you also have ONE dbus-daemon hanging around?

@KenT2 I am running your omx.sh script. I assume that the 'omxplayer' started is the omxplayer script.

So, how is it possible to run the script WITH the forking, and does running the script causes the multiple dbus-daemons?

I can live with just ONE dbus-daemon hanging around ;)

@KenT2
Copy link

KenT2 commented Jan 18, 2014

I have always assumed my script shown in #124 runs the script and NOT omxplayer.bin. If not I have been doing an unintended thing for 18 months :-(

@jehutting
Copy link

Please don't understand me wrong. I'm not saying one shouldn't use the script! I only find it strange why I can't use the script and also like to know if the script really kills the forked daemon, so ZERO daemon hanging around.

In my (very little) understanding of D-Bus, the dbus-launch-er starts the dbus-daemon, and providing so a D-Bus interface to omxplayer. But then, shouldn't the dbus-launch-er also be responsible for killing the daemon? Is it correct to be stuck with ONE daemon? Also when omxplayer is started again, how does the launcher knows it is omxplayer again and use the 'hanging around' daemon in stead of starting a new one?

Again, I can live with the one hanging around daemon -doesn't causes any harm (yet :) but should it be?

@KenT2
Copy link

KenT2 commented Jan 18, 2014

Just to clarify. With the script in #124 you do not get orphan dbus-daemons because the daemon is killed of by CTRL-C used to get back to the command line. Sorry for misleading you.

Orphan daemons occur when testing with Pi Presents. This is probably because I am not killing the omxplayer script correctly. Does anyone know the proper way to dispose of the omxplayer script such that the daemons and omxplayer.bin also get removed. Ideally I would like this to be by the using the PID of the omxplayer script returned by pexpect as I would like eventually to have mooe than one copy of omxplayer running.

@andyjagoe
Copy link

@jehutting Yes, I have one dbus-daemon hanging around. I'm using omxplayer via python subprocess...and perhaps I'm not killing it properly either and have an issue similar to @KenT2 . My code is below and you can see the timer...though most times the EOS from omxplayer ends the subprocess. Would love any thoughts or pointers on how to do this better.

        def omxplayer_monitor(proc):
            Logger.debug('Stopping omxplayer: omxplayer_monitor reached timeout value')
            os.killpg(proc.pid, signal.SIGTERM)

        #set up our subprocess
        p = subprocess.Popen( ['omxplayer', 
                       '-s',
                       '-g',
                       '-o',
                       'hdmi', 
                       item['path']
                       ], 
                      stdin = subprocess.PIPE, 
                      stdout = subprocess.PIPE, 
                      close_fds = True,
                      preexec_fn=os.setpgrp #this allows us to kill omxplayer via omxplayer_monitor
                      ) 

        #after n seconds, kill video process. Replace n with actual length of video once we get duration value
        t = 360.0
        try:
            t = float(item['duration'])
            #provide 10 seconds of extra slush time to make sure we don't quit videos early if they take a few seocnds to launch
            t = t + 10.0 
        except Exception, err:
            Logger.error('ERROR: %s\n' % str(err))            

        args = p,
        monitor = threading.Timer(t, omxplayer_monitor, args=(args))
        monitor.start() 

        #run it and get results
        stdout_text, stderr_text = p.communicate()

        #cancel our monitor just in case
        monitor.cancel()

@KenT2
Copy link

KenT2 commented Jan 18, 2014

@andyjagoe

Googling a bit, including here http://www.tutorialspoint.com/unix/unix-signals-traps.htm it looks like we should be sending SIGINT rather than SIGTERM or SIGKILL. This gives the omxplayer script a chance to tidy up.

Having said that I cannot see anything in the omxplayer script to catch the SIGINT. Maybe there is something magic that means trap is not required or CRTL C from LXTerminal does some special magic. I'll experiment tomorrow.

@jehutting
Copy link

Thanks @andyjagoe and @KenT2 for the feedback.

So if I summarize,

  • the orphan dbus-daemons occure ONLY when the omxplayer script is killed.
  • after a successful play there is ONE (orphan?) dbus-daemon left.

With killing is meant really killing, just abrupt ending omxplayer and/or omxplayer script. I can imagine that this makes them unable to do some house cleaning stuff; allocated space, OMX core components resources not being released, and the forked dbus-daemon not being notified that it should die. How would a bash script be able to do so? Is there a signal trap handling? Don't think there is something automatically done for that. Also, there will be a point where the OS runs out of resources (memory). Don't think there is such a thing as a garbage collector, or is there nowadays?

The ONE dbus-daemon should it be there? Still have the feeling it shouldn't. Would be interesting to know if WE ALL have this ONE dbus-daemon left over. SO if one has some spare time, use 'ps xaf | grep -i dbus' and let me know. Just for fun!

Besides this D-Bus problem, I think that all those orphans is caused by that other issue which I'm looking at: the hangups. THAT issue is the reason why you guys need to kill omxplayer. It is simply a work around :-)

Sending a signal SIGTERM SIGINT is more likely to prevent the orphans. Now omxplayer will be able to do the house cleaning; omxplayer's signal handler raises g_abort on which all internal stuff should shutdown/end their processes. When omxplayer wished you 'have a nice day', the omxplayer script is on it's turn able to terminate (by kill -2 = SIGTERM) the forked D-Bus daemon.

Andyjagoe, I see that the monitor does send a SIGTERM. Event with this, omxplayer does NOT wish you a nice day? Would be interesting too.

@KenT2
Copy link

KenT2 commented Jan 19, 2014

@jehutting
Thanks for that very helpful explanation. I am trying to SIGINT the omxplayer script at the moment to tidy up from crashes.

I notice that when I boot Raspbian even before running omxplayer that there is a dbus-daemon and dbus-launch when I look at the processes with top -upi. I wonder if this is your leftover daemon?

@andyjagoe
Copy link

Thanks all for the feedback and research. I can experiment with sending SIGINT instead of SIGTERM.

One thing to note in my code is that in most cases subprocess receives EOS and is not actually killed via the monitor. I'll have to look at the logs...but not sure whether receiving 'have a nice day' indicates dbus-daemons were successfully shut down.

I agree with @KenT2 that the dbus-daemon running might be present prior to omxplayer and not related...

@jehutting
Copy link

Hi @KenT2 and @andyjagoe

What I see BEFORE starting omxplayer is

$ ps xaf | grep -i dbus
 2223 ?        Ss     0:00 /usr/bin/dbus-daemon --system
 2604 pts/0    S+     0:00              \_ grep --color=auto -i dbus

and AFTER omxplayer is started (and ended) is

$ ps xaf | grep -i dbus
 2223 ?        Ss     0:00 /usr/bin/dbus-daemon --system
 2648 pts/1    S+     0:00              \_ grep --color=auto -i dbus
 2619 pts/0    S+     0:00 dbus-launch --autolaunch 355ec9f99dfe1d2519ffc50d508db147 --binary-syntax --close-stderr
 2620 ?        Ss     0:00 /usr/bin/dbus-daemon --fork --print-pid 5 --print-address 7 --session

So yes there is a dbus-daemon already running but that is the SYSTEM dbus-daemon (noticeable by --system). Omxplayer (or something) starts the dbus-launch, which starts the SESSION dbus-daemon (noticeable by --session). That is what I make of the D-Bus process so far.

I'm running only from the command line (ssh) and not from X windows (GUI). Maybe there is a difference. I don't know.

@KenT2
Copy link

KenT2 commented Jan 19, 2014

@jehutting

This is what I get before and after running omxplayer. SSH is enabled but not in use. I am using X and LXDE. uncluttter is running.

pi@raspberrypi ~/pp_home/media $ ps xaf | grep -i dbus
1936 ? Ss 0:00 /usr/bin/dbus-daemon --system
2235 ? Ss 0:00 _ /usr/bin/ssh-agent /usr/bin/dbus-launch --exit-with-session x-session-manager
2238 ? S 0:00 /usr/bin/dbus-launch --exit-with-session x-session-manager
2239 ? Ss 0:00 /usr/bin/dbus-daemon --fork --print-pid 5 --print-address 7 --session
2436 pts/0 S+ 0:00 _ grep --color=auto -i dbus

pi@raspberrypi ~/pp_home/media $ omxplayer xthresh.mp4
Video codec omx-h264 width 720 height 576 profile 77 fps 25.000000
Audio codec aac channels 2 samplerate 32000 bitspersample 16
Subtitle count: 0, state: off, index: 1, delay: 0
V:PortSettingsChanged: 720x576@25.00 interlace:0 deinterlace:0
dest_rect.x_offset 0 dest_rect.y_offset 0 dest_rect.width 0 dest_rect.height 0, pixel_aspect 1.07
have a nice day ;)

pi@raspberrypi ~/pp_home/media $ ps xaf | grep -i dbus
1936 ? Ss 0:00 /usr/bin/dbus-daemon --system
2235 ? Ss 0:00 _ /usr/bin/ssh-agent /usr/bin/dbus-launch --exit-with-session x-session-manager
2238 ? S 0:00 /usr/bin/dbus-launch --exit-with-session x-session-manager
2239 ? Ss 0:00 /usr/bin/dbus-daemon --fork --print-pid 5 --print-address 7 --session
2462 pts/0 S+ 0:00 _ grep --color=auto -i dbus

pi@raspberrypi ~/pp_home/media $

@jehutting
Copy link

@KenT2 Thanks for the output.
Sorry, but I was not very clear about what I meant with 'BEFORE omxplayer'. BEFORE is before running omxplayer the first time after a Raspberry Pi startup. I see that the dbus-launch and dbus-daemon just before and after omxplayer run are the same (having the same pid). I bet that these are started by running omxplayer but not killed by omxplayer (or the one who initiated the dbus-launch, it is not clear to me). I'm pretty sure that this daemon stays there forever.

@KenT2
Copy link

KenT2 commented Jan 20, 2014

The first ps WAS before running omxplayer for the first time. i.e. immediately after reboot

@KenT2
Copy link

KenT2 commented Jan 20, 2014

@jehutting

And thanks. Sending a SIGINT to omxplayer script worked wonderfully. I have had three crashes of omxplayer in a run and no orphan dbus-daemons. :-)

@jehutting
Copy link

@KenT2, again thanks for the feedback...so I lost my bet... ;)))))

Nice too see that SIGTERM SIGINT works. Nice job.

I'm still hunting for the crashes, but I'm a little bit stuck on the OMX_ErrorInsufficientResources as mentioned in issue 124 and 12.

@jehutting
Copy link

I also ran ps xaf | grep -i dbus on the GUI. Got the same result as KenT2 (see some comments above).

So my conclusion (from what I understand untill now about D-Bus, and yes I have to read more ;) would be:

  • As there is already a session dbus-daemon active before omxplayer has run (for the first time), and that that one is the ONLY one during the time omxplayer is running and also still is the ONE AND ONLY after omxplayer stopped playing, there is ONLY ONE SESSION dbus-daemon for ALL applications using a SESSION connection.
  • There is no need to fork a dbus-daemon in order to run omxplayer. Commenting out the dbus stuff in omxplayer script shows this (see above andyjagoe mods and issue writing bytes to omxplayer's OutputStream from java #116). Also detected in issue writing bytes to omxplayer's OutputStream from java #116 there shouldn't be a forked/second session dbus-daemon as this results in mdbus2 failing to detect omxplayer is on the D-Bus. (otherwise I would expect that mdbus2 would be capable to handle multiple dbus-daemons, and that it finds omxplayer :)))

Anyone any comment on this?

@blakee
Copy link

blakee commented Feb 11, 2014

My opinion is that there should only be one D-Bus session bus created, that's discoverable at some well-known location instead of a mktemp'ed file. All omxplayer instances should share it.

The MPRIS spec says "In the case where the media player allows multiple instances running simultaneously, each additional instance should request a unique bus name, adding a dot and a unique identifier to its usual bus name, such as one based on a UNIX process id."

There is no need to fork a dbus-daemon, this is true, but if omxplayer can't get onto a session bus you won't be able to use keyboard controls.

@popcornmix
Copy link
Owner

@blakee
I'd be interested in a more correct implementation with a single D-Bus session.

@jehutting
Copy link

So the bus name for each MPRIS instance (omxplayer) should be unique. Seems reasonable, but when this name is made from the process id (just like mktemp seems to do) isn't it than for a client hard to find out which name belongs to which instance?

@blakee
Copy link

blakee commented Feb 12, 2014

We could have an instance id that gets passed as a command-line argument to omxplayer, and causes omxplayer to attempt to register org.mpris.mediaplayer2.omxplayer.{instance id} on dbus. If no instance id is specified, it will first attempt to register org.mpris.mediaplayer2.omxplayer, if that fails it will use org.mpris.mediaplayer2.omxplayer.instance{pid}.

My reasoning is that when there is only one instance of omxplayer running at once (the common case, I believe) the user doesn't have to specify an instance id, and can still use any dbus client pointed at org.mpris.mediaplayer2.omxplayer. If you are trying to juggle multiple players at once, you probably have your own unique identifier you can use, so just pass that.

@popcornmix
Copy link
Owner

@Aiexis
Is this still an issue with latest omxplayer and single dbus interface?

@Aiexis
Copy link
Author

Aiexis commented Feb 21, 2014

@popcornmix it seems that dbus process is now correctly killed. Thank you ;)

@popcornmix
Copy link
Owner

Good to hear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants