Dup 2 Permissions Error on Lab Machines

[JustinMeimar]

Bug Description:

• Student found an error for which dup2 would complain when redirecting the input and output contents to the current commands stdin and stdout.

Root Cause:

• Problem was due to use of fs::temporary_directory which started to cause name collisions in the tmp directory between students on the lab machines who use the same command step name.

• For example, changing generator-interpreter to generator-interpreter-changed-to-no-conflict in the config below resolved problems.

{
  "testDir": "./testfiles",
  "testedExecutablePaths": {
    "kuknur": "../bin/generator"
  },
  "toolchains": {
    "interpreter": [
      {
        "stepName": "generator-interpreter-changed-to-not-conflict",
        "executablePath": "$EXE",
        "arguments": [
          "$INPUT",
          "$OUTPUT"
        ],
        "output": "generator.out"
      }
    ]
  }
}

I am going to be refactoring how temporary files are created, named and managed in the process of fixing this bug to reduce the overall complexity.

[kuzi117]

I remember dealing with this and ending up with the solution that was there previously for a bunch of reasons. Annoying thing about this is that you have to consider if/when the temp directory is getting cleaned up.

• Build up: Run 100 tests, each with 2-3 steps, get 200-300 files (since you're not going to be overwriting anymore). It's not a ton of output for each test (except .ll files) but it's not zero and that can be a concern. Consider also that every tester run will generate the same file count and I can imagine running my tests potentially tens of times per night of work.
  • /tmp is local: I'm fairly certain /tmp is on the local disk, not the network, which (pro) I think means it doesn't count against their network storage, but (con) IST gets annoyed if you use all of the local disk. Less of a concern if they're using (or ssh'ing to) a lab machine, but doing it on ohaton or something will bring their ire.
  • /tmp is shared: /tmp is not a user specific folder, so if any other user is running the tool, they'll also be filling the drive up with several hundreds of files. More storage concerns.
  • Clean up: A quick search shows that many *nix distros (who knows about windows) clear /tmp on restart. IST has probably implemented extra rules on wiping /tmp. Either way, it's a boon and a curse: nightly clean ups might mean you don't have to worry about accumulating files but it means your results could disappear from under you, though you can just run again so it doesn't suck that much. Never cleaning up means it lasts for ever.. but lasting forever means the tester or the user needs to clean up after themself.
• Discovery: Having the tester have one file per test and having it die on a failure meant that the files in your local directory are guaranteed to be related to your issue. Random names for each file (e.g. <stepname>.txt.<uuid> or something) will make just a sea of files that can be difficult to parse.
  • /tmp is still shared: /tmp is now going to have files that aren't your own as well and you'll be stuck sifting through even more files to find the one you're looking for.

Solutions that could work but kinda suck:

• Clean up after yourself: Have the tester track all generated file names and delete them when you're done.
  • Pro: No file build up. Temp is clean.
  • Con: You never get to see the result files, only diffs.
    • You could have a switch that disables the clean up mechanism, but that just reintroduces everything above, especially since I know someone is going to write tester .... --no-cleanup once and just keep hitting up to rerun that command regardless of warnings to the contrary.
    • You could die on test failure and yank the relevant files to the current working directory and rename them to the "right" name, so that users can investigate. This still lets you clean up behind you. Doesn't really let you nicely implement continue-after-failure though since you still need to figure out how to have multiple copies of error-related files.
• Revert to local files: Doesn't play well with multithreading that's on its way.

In any case, I'm not sure how much you're going to be able to reduce complexity, especially now that I remembered that this is going to have to work with multithreading.

[JustinMeimar]

Thanks for the quick feedback. I'm experimenting with creating a directory in the users FS adjacent to AssignmentConfig.json and doing the self-cleanup in each Testfile / Command destructor. It might be nice to get away from the shared /tmp entirely.

While self-cleanup does eliminate the ability to look back at generated files for debug info, there is the -vvv triple verbosity mode now which I think we could just pack with relevant debug info while the program is running and resources are still alive.

I think the crux is as you mentioned, that the file build up will get very large for N tests x M steps per tool-chain. If we re-use the same file (overwriting each time to avoid many files) this is not able to be multi threaded. So my current approach is now to try unique file names to keep multi-threading possible + self-cleanup of all generated files + private tmp directory to keep multiple users isolated.

Another idea that just occurred to me which might work. Using a thread ID as a suffix to temporary files that are otherwise always rewritten. This might find a balance between creating a unique intermediate file for each tool-chain step and rewriting the same filename each time?

Thanks for your thoughts @kuzi117