Up to Main Index                          Up to Journal for November, 2018

                    JOURNAL FOR FRIDAY 16TH NOVEMBER, 2018
______________________________________________________________________________

SUBJECT: Of data races, vetoes, junking and barriers
   DATE: Fri 16 Nov 20:42:21 GMT 2018

A QUICK WARNING! If you use the GET or TAKE commands with $ACT make sure the
actor has an inventory defined. Otherwise you will get a nil panic. An
automatic fix for this still needs to be worked out. I’ll probably modify the
GET and TAKE commands to check if the actor has an inventory…

While busy working on adding barriers I managed to trigger a data race in some
unrelated code. It took a while to figure out what was going wrong, especially
as I wasn’t logging to disk at the time the data race occurred.

During initialisation, the zone loader calls checkDoorsHaveOtherSide to create
the ‘other side’ of a door. A door actually consists of two doors in different
locations but with a shared state. For example, we have the ‘Tavern entrance’
and going east takes us to the ‘Street between tavern and bakers’. The tavern
door is defined as being in the entrance and blocks the east exit when closed.
During initialisation calling checkDoorsHaveOtherSide follows the door’s exit
to ‘Street between tavern and bakers’ and creates the ‘other side’ which leads
back west into the tavern. Having two linked doors is easier than one door
that straddles two different locations. You can then examine the door in
either location and open or close it from either location. It also means that
you can have the two doors on opposite sides of the world and still walk
between them, aka portals :)

What does this have to do with the data race? Turns out I wasn’t locking the
destination location before creating the ‘other side’ of the door when loading
zones and initialising the world.

I’m still running some tests, which will take a while as this is a VERY hard
bug to reproduce. How do you test to see if something isn’t there any more or
just hasn’t occurred yet? However, I believe my fix is correct and the data
race has been eliminated. A simple mistake, hours of debugging fun…

In other news, as I said I’ve been working on barriers. To get barriers
working I’ve had to change how veto checks are done. As a result, vetoes.Check
now takes a has.Thing as the first parameter. This should be the actor for the
command being checked. This change makes the veto checks more interesting, as
checks can use information gleaned from the actor: where they are, who they
are, items carried and any other information that is available through the
actor. This is required for barriers — we need to find the aliases of the
actor to test them against the allow/deny lists for the barrier to see if the
actor is allowed to pass through the barrier or not.

Existing instances of calls to vetoes.Check have been updated, as has the
has.Vetoes interface.

While working on the veto changes I noticed a bug when junking containers. I
traced the bug back to commit eb7aab063302ae36:


  cmd: Clean up comments and code for junk.vetoed


When junking an item any content is checked to make sure all items can be
junked. The above commit broke these checks by terminating the recursive
checks too early — recursive checks as we can have containers inside
containers. As a result not all items in a container were being checked. This
resulted in some items sometimes being junkable when they shouldn’t have been.
An example is putting the lattice in the chest and junking the chest. Poof!

The broken veto checks have been fixed in cmd/junk.go for the junk.vetoed
method.

Yet another bug I discovered was that $RESET would sometimes throw a nil
panic. This is proving to be another illusive and hard to reproduce bug.

The public dev branch has been updated with these fixes and changes so far:


  - attr,cmd: $CLEANUP should use actor and not search for aliases
  - cmd: Fix vetoed returning early causing non-junkables to be junked
  - attr,has,cmd: Pass actor to has.Vetoes.Check method
  - zones: Fix data race by locking destination in checkDoorsHaveOtherSide
  - cmd: Use text.TheName for DROP, EXAMINE & SAY commands
  - attr: Fix variable naming nit in Door.Found


The issues with $RESET I’m still working on. Also working on a fix for the
$ACT command with GET/TAKE issues. I’ve not committed my work on barriers yet.
Hopefully that will happen over the weekend.

With all of these fixes, especially for the data race and panics, I might just
have to do the next release sooner rather than later.

--
Diddymus


  Up to Main Index                          Up to Journal for November, 2018