[MPlayer-dev-eng] [PATCH] watchpid slave command

[Nicolas George]

Your mailer breaks threading and sends overly long lines: you should fix its
settings.

Le quintidi 25 thermidor, an CCXVII, Emmeran Seehuber a écrit :
> Yes this is true, there can be races. But usally the same PIDs are not
> given out again so fast. So I think/hope this is only a theoretical race.

If you are watching a long movie or listening to a long playlist, the PIDs
have plenty of time to wrap around.

> The problem is that this not only affects slave input, it also affects
> slave output. At least on Windows mplayer blocks forever if it tries to
> write to a pipe which other side is no longer reading (because it is
> gone). Maybe on Linux mplayer would get a SIGPIPE here, but at least on
> Windows it does not get this and just plain simply hangs :( 
> 
> So it would be nessesery to check not only on input but also in mp_msg().
> I´m not sure that a simple feof() checking before doing the fprintf()
> would be enough here. But I´ll try that when I find time.

You do not have to test for EOF on writing. In your patch, you have a call
to kill(pid, 0) to know if a process is still alive. This call can be
replaced by a test of the input socket: it will give exactly the same
results, apart from being immune to the problem of PID wrap.

If, as you say, writing on a dead socket on windows is blocking (which is
fairly braindead; could you not use a different type of socket with a saner
behaviour?), then any approach that tests periodically the presence of the
parent process has an inherent race condition.

To lower the impact of this race condition, you seem to have put calls to
check_watched_process almost everywhere. I find that it makes an ugly patch:
a very minor feature ends up with code all over the place.

I would advice for the following course:

- Use the EOF condition on input to know that your parent process has died.

- If the test is not frequent enough, add calls to the input subsystem at
  strategic places (mp_msg seems a good place; but beware recursive calls).

You have the benefit to make your code more portable (no need for
GetExitCodeProcess: mplayer already calls a function when it sees the EOF)
and more readable (because code related to that particular feature will be
in only one place).

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20090812/fc0af593/attachment.pgp>