On 2022/02/10 08:42, Marc Espie wrote:
> On Wed, Feb 09, 2022 at 07:30:46PM -0800, Andrew Hewus Fresh wrote:
> > I was reminded that fw_update(8) updates the package database without
> > locking currently. That can cause issues when running it concurrently
> > with pkg_add, for example starting `pkg_add -u` in one terminal and
> > `sysupgrade` in another.
> >
> > This diff checks to see if perl is available and if so starts a perl
> > co-process that locks the database and then kills it when exiting.
> >
> > (Figuring out how to wait for the lock to be acquired and find out the
> > pid of the co-process was a bit of a challenge, so if someone has a
> > suggestion on improving that, I'm open to suggestions)
> >
> > Comments, OK?
> >
> >
> > Index: usr.sbin/fw_update/fw_update.sh
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
> > retrieving revision 1.36
> > diff -u -p -r1.36 fw_update.sh
> > --- usr.sbin/fw_update/fw_update.sh 10 Feb 2022 00:29:32 -0000 1.36
> > +++ usr.sbin/fw_update/fw_update.sh 10 Feb 2022 03:22:20 -0000
> > @@ -42,11 +42,13 @@ INSTALL=true
> > LOCALSRC=
> >
> > unset FTPPID
> > +unset LOCKPID
> > unset FWPKGTMP
> > REMOVE_LOCALSRC=false
> > cleanup() {
> > set +o errexit # ignore errors from killing ftp
> > [ "${FTPPID:-}" ] && kill -TERM -"$FTPPID" 2>/dev/null
> > + [ "${LOCKPID:-}" ] && kill -TERM -"$LOCKPID" 2>/dev/null
> > [ "${FWPKGTMP:-}" ] && rm -rf "$FWPKGTMP"
> > "$REMOVE_LOCALSRC" && rm -rf "$LOCALSRC"
> > [ -e "${CFILE}" ] && [ ! -s "$CFILE" ] && rm -f "$CFILE"
> > @@ -194,6 +196,36 @@ firmware_devicename() {
> > echo "$_d"
> > }
> >
> > +lock_db() {
> > + [ "${LOCKPID:-}" ] && return 0
> > +
> > + # The installer doesn't have perl, so we can't lock there
> > + [ -e /usr/bin/perl ] || return 0
> > +
> > + set -o monitor
> > + perl <<'EOL' |&
> > + use v5.16;
> > + use warnings;
> > + use OpenBSD::PackageInfo qw< lock_db unlock_db >;
> > + use OpenBSD::State;
> > +
> > + $|=1;
> > +
> > + lock_db(0, OpenBSD::State->new);
> > + END { unlock_db }
> > + $SIG{TERM} = sub { exit };
> > +
> > + say $$;
> > + sleep;
> > +EOL
> > + set +o monitor
> > +
> > + read -rp LOCKPID
> > +
> > + return 0
> > +}
> > +
> > +
> > installed_firmware() {
> > local _pre="$1" _match="$2" _post="$3" _firmware _fw
> > set -sA _firmware -- $(
> > @@ -401,6 +433,7 @@ set -sA devices -- "$@"
> >
> > if "$DELETE"; then
> > [ "$OPT_F" ] && echo "Cannot use -F and -d" >&2 && usage
> > + lock_db
> >
> > # Show the "Uninstall" message when just deleting not upgrading
> > ((VERBOSE)) && VERBOSE=3
> > @@ -463,6 +496,8 @@ else
> > fi
> >
> > [ "${devices[*]:-}" ] || exit
> > +
> > +lock_db
> >
> > added=''
> > updated=''
> >
> It's one of the cases where you can actually use the newer OpenBSD::BaseState
> where I explicitly split the printing and system functions.
>
> e.g.,
> use OpenBSD::BaseState;
>
> lock_db(0, 'OpenBSD::BaseState');
> should work just as well
>
That also works for me.
