PiDP-8/I Software

Possible mmake issue: no lock on use of scratch files.
Log In

Possible mmake issue: no lock on use of scratch files.

(1) By Bill Cattey (poetnerd) on 2018-11-01 03:50:51 [link] [source]

On my PiDP-8/i, I ran tools/mmake to try out my latest build. I got a failure:

OSError: [Errno 2] No such file or directory: '/home/pi/src/pidp8i/os8-v3f-extensions/bin/v3d-copy.rk05'
Makefile:572: recipe for target 'bin/v3f-td12k.tu56' failed
make: *** [bin/v3f-td12k.tu56] Error 1

In looking at the mmake output earlier, it was clear that it was simultaneously building the tc08 and td12K tape image. The log file interleaves output from the two builds:

Running script file: ./media/os8/scripts/v3f-control.os8
copy command: 
    from: /home/pi/src/pidp8i/os8-v3f-extensions/obj/v3f-build.rk05, 
    to: /home/pi/src/pidp8i/os8-v3f-extensions/bin/v3f-made.rk05
bin/os8-run ./media/os8/scripts/all-tu56.os8 --enable v3f
bin/os8-run ./media/os8/scripts/all-tu56.os8 --enable v3f --enable td12k
WARNING: Failed to lock GPIO for exclusive use.  Won't use front panel.
Running script file: ./media/os8/scripts/all-tu56.os8
Building v3f
WARNING: Failed to lock GPIO for exclusive use.  Won't use front panel.
Running script file: ./media/os8/scripts/all-tu56.os8
Building v3f
for td12k configuration of TD8E.
with TC08 support.
Traceback (most recent call last):
  File "bin/os8-run", line 222, in <module>
    main()
  File "bin/os8-run", line 213, in main
    os.remove(filename)
OSError: [Errno 2] No such file or directory: '/home/pi/src/pidp8i/os8-v3f-extensions/bin/v3d-copy.rk05'
Makefile:572: recipe for target 'bin/v3f-td12k.tu56' failed
make: *** [bin/v3f-td12k.tu56] Error 1

The script DOES create a v3d-copy.rk05 when building for td12k (but not for tc08):

begin enabled td8e
  mount rk0 $bin/v3d.rk05 required scratch

The "scratch" is what creates the file. It **HAS ** to exist.

I'll dig into os8-run and see if it deletes all scratch files on exit regardless of whether or not it was created on that run. But I THOUGHT I kept a list of scratch files created, and only delete ones I actually created.

Nevertheless, if there were a v3d td12k and a v3f td12k build happening simultaneously, the scratch file would be trashed by the other.

Perhaps the correct fix is to add a uniquifier to the scratch file names to prevent collisions and damage from simultaneous operations.

(2) By Bill Cattey (poetnerd) on 2018-11-01 03:52:55 in reply to 1 [link] [source]

Actually, there seems to be something else not quite right with the creation of the v3d-td12k.tu56 images. For some reason the script seems quite happy to exit just after the include of cusp-copyin.os8 finishes.

The v3f pass through doesn't do that.

I see that I also have a typo in the utilities copied in and the tc08 tape utilities are copied in when td8e ones should.

I'll fix this tomorrow. Bed time now.

(3) By Warren Young (tangent) on 2018-11-01 17:16:56 in reply to 2 [link] [source]

The "scratch" modifier could cause os8-run to use tempfile internally, so that you can have multiple non-interfering scratch copies at once.

(4) By Warren Young (tangent) on 2018-11-01 17:22:44 in reply to 3 [link] [source]

More generally, the build system should be purely functional: given the same input, it should always produce the same output, regardless of the way the operations in parallel build streams are interleaved.

The fact that OS/8 modifies its disks just by reading them means it isn't functional in this sense, so we have to fork the history of the file before sending it to the input of each build stream.

Doing this will also avoid the need for locks: the file we're copying won't change after being generated, and we don't care that the scratch copy gets changed during each independent build process.

Yes, it's expensive in terms of disk space and I/O time, but it's also mathematically robust.

(5) By Bill Cattey (poetnerd) on 2018-11-02 03:29:44 in reply to 3 [link] [source]

I've checked in code for the scratch option to use tempfile as you recommended.

I have not thought very hard about what it means when v3d.rk05 is running in multiple sub-builds.

We may have to set scratch for every script.

(6) By Bill Cattey (poetnerd) on 2018-11-02 03:34:39 in reply to 4 [link] [source]

There's another race condition that I just tripped over when running mmake on the pi:

OSError: [Errno 2] No such file or directory: '/home/pi/src/pidp8i/os8-v3f-extensions/media/os8/init.cm.pt_temp'

The init.cm file is copied in by several scripts which in mmake are running simultaneously.

So ALL temp file creation needs to use tempfile.

Bed time again.

This weekend is my Chorus's fall retreat. I won't be able to work on more of this until late Sunday afternoon.

(7.1) By Bill Cattey (poetnerd) on 2018-11-03 03:23:17 edited from 7.0 in reply to 6 [link] [source]

I had a brief moment to research the issue.

Root cause is in simh.py: os8_pip_to and os8_pip_from. In creation of temp filename for the /A option that converts POSIX to OS/8.

The name is passed to ptconv tool.

Use of tempfile.MakeNamedTemporary won’t work because we are giving a name, not opening a file.

Probably salting the name with a process ID will be sufficient.

(8) By Bill Cattey (poetnerd) on 2018-11-05 05:52:55 in reply to 7.1 [link] [source]

I have updated os8_pip_to and os8_pip_from to add the PID to the temporary name.

I considered using a randomly generated UUID, but felt it was overkill.

mmake now works on both my Mac and my Pi.

However, I think a lock file will need to be used for os8-cp because:

os8-cp might be used to modify files on "The packs you always use." So it can't create a scratch image, lest its work disappear after it runs.

However, we might be using os8-cp during a parallel make. At the present time we use it exactly once, in creating v3f-build.rk05. (Note that the Makefile variable OS8CP is defined twice in Makefile.in. I think one instance should go away.)

Any thoughts on what to do here?

(9) By Bill Cattey (poetnerd) on 2018-11-05 21:24:36 in reply to 5 [link] [source]

Current status:

Script files have been updated to use scratch were sensible.

all-tu56.os8, v3d-src.os8, v3f-control.os8 expected to run in parallel. They now use scratch.

v3d-dist.os8, v3d-rk05.os8 change the boot image and cant use scratch. They must be serialized in Make.

os8-cp has problem because it might edit the boot image but might be run at the same time as something else. This probably needs a lock file.

(10) By Warren Young (tangent) on 2018-11-06 03:51:37 in reply to 9 [link] [source]

Sometimes this can be achieved with careful dependencies, which purposely choke make's choices down to just one in the critical section.

See, for example, how $(INFILES) are handled: we purposely avoid listing all of the output files here because that would give make the freedom to try and create them in parallel, which in the autoreconfigure case means re-running the configure script in parallel, which risks partial overwrites and other badness. So, we give make only one choice through this path, which ensures that the configure script can be re-run automatically only once at most per build cycle, regardless of thread count.

(11) By Bill Cattey (poetnerd) on 2018-11-08 22:04:16 in reply to 10 [source]

Acknowledged.