[systemd-devel] Using *.path units without races?

[Uwe Geuder]

Hi!

On Tue, 17 Mar 2020 at 23:52, Michael Chapman wrote:
>
> On Wed, 18 Mar 2020, Uwe Geuder wrote:
> > Hi!
> >
> > I have wondered for a while how I can use *.path units without (too bad)
> > races.
> >
> > Since
> > https://github.com/systemd/systemd/pull/13509/commits/06582e42de65a61d0238a18720a12b6353edb7cd
> > the behaviour has been become much clearer, but I must admit I still
> > don't get it.
>
> That commit does look incomplete to me.
>
> As a quick test, are you able to try out the patch below? This makes
> systemd always check the filesystem when the service stops, rather than
> just relying on the (as of that commit nonexistent) inotify event.
...
> diff --git a/src/core/path.c b/src/core/path.c
> index cb75d778af..a513df97b2 100644
> --- a/src/core/path.c
> +++ b/src/core/path.c
> @@ -759,11 +759,7 @@ static void path_trigger_notify(Unit *u, Unit *other) 
> {
>          if (p->state == PATH_RUNNING &&
>              UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
>                  log_unit_debug(UNIT(p), "Got notified about unit deactivation.");
> -
> -                /* Hmm, so inotify was triggered since the
> -                 * last activation, so I guess we need to
> -                 * recheck what is going on. */
> -                path_enter_waiting(p, false, p->inotify_triggered);
> +                path_enter_waiting(p, false, true);
>          }
>  }
> 

Thanks a lot Michael for this quick patch. The patch seems to have
suffered a formatting accident in the mail. However, your intent to
change the 3rd agument of that function call is pretty clear.

I built that change and quickly tested it. It seems to work fine!

Please note that I have not yet done any wider (regression) testing and
my understanding of the code base is not good.

I can share my test code

----- test.path -----
[Unit]
Description=%n testing racyness of path

[Path]
DirectoryNotEmpty=/tmp/systemd-path-test/
MakeDirectory=true


----- test.service -----
[Unit]
Description=%n testing racyness of path

[Service]
ExecStart=%h/try/systemd/test.sh


----- test.sh -----
#!/bin/sh -eu
# Usage: Expects to find <number>.sleep files in ${testdir}

echo "${0} starting"
testdir=/tmp/systemd-path-test/

exit_handler() {
  echo "${0} exiting"
}
trap exit_handler EXIT

# assume good faith, don't worry about dotfiles or
# pathologic filenames...
files=$(ls "${testdir}/")
if [ -z "${files}" ] ; then
  echo "No files found"
  exit
else
  echo "Found ${files}"
fi

for f in ${files} ; do
  if [ -e "${testdir}/${f}" ] ; then
      rm -f "${testdir}/${f}"
  else
    echo "Race: ${f} has disappeared"
  fi
  case "${f}" in
    *.sleep) # don't worry about non-numeric...
      s=$(basename "${f}" .sleep)
      echo "sleeping ${s}"
      sleep "${s}"
      ;;
  esac
done

------

Test case:

touch /tmp/systemd-path-test/5.sleep ; sleep 2 ; \
  touch /tmp/systemd-path-test/6.sleep ; sleep 5 ; \
  touch /tmp/systemd-path-test/0.5.sleep

(all 5 commands in one go)

As expected the service gets now invoked 3 times. Without your patch the
second touch command/file is missed and only "handled" together with the
third touch command/file.


Regards,

Uwe

Uwe Geuder
Neuro Event Labs Oy
Tampere, Finland
uwe.gxuder at neuroeventlabs.com (bot check: fix 1 obvious typo)