[PATCH] D153771: [BOLT][Instrumentation] Fix hash table memory corruption and append-pid option

[Denis Revunov via Phabricator via llvm-commits]

treapster added inline comments.


================
Comment at: bolt/test/runtime/instrumentation-indirect-2.c:111
+# Wait till output is fully written in case child outlives parent
+RUN: sleep 1
+# Make sure all functions were called
----------------
rafauler wrote:
> Do you really need that? (Lines 111 and 66)
> 
> I'm testing locally with the sleep timers removed and the test is not failing. If line 65 " Wait for profile and output to be fully written"  is the problem, I don't know, perhaps something like https://linux.die.net/man/1/sync?  (I don't know because I'm not sure what's happening in your machine, but it looks odd to me that the process will finish but your test script will access a half-finished output file).
> 
> If the problem is the one described in line 110 "in case child outlives parent", then perhaps using bash's  "wait" command? https://phoenixnap.com/kb/bash-wait-command
I do get spurious failures if i remove any of the sleeps, and in the second case it always looks like incomplete output. Wait only works for a background job of the shell, in our case we have the main process finished and the output file read by next commands before the child finishes and writes it's output. AFAIK there is no way for us to wait for the whole process tree so we have to just sleep and hope it's finished. Regarding sync, it is also not what we need because we don't care whether the file is flushed to disc or not, we just need to wait till a process stops writing to it. Although sync seems to be working, probably because it spends enough time flushing for child process to finish and write everything. But still can fail any time.
There is another solution, however: for stdout redirection, i imagine we'll have the file open till both processes finish. So we can wait in a loop until `fuser` returns 1, which will guarantee both processes finished. For instrumentation profile, we'll need to change watchProcess() so that it opens profile only once and keeps FD open until it's done, seeking to zero on every iteration. This way we can also query `fuser` on the file to know when it's safe to read it. But this is a topic for another diff:).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153771/new/

https://reviews.llvm.org/D153771