Race condition on history file with multiple REPLs

[rvagg]

via @substack

<substack> found another repl history bug, if you have 2 repls open at once it doesn't lock the file
<substack> but a bit hard to reproduce since they've got to be writing at the same time
<substack> but I already ran into it just doing normal things with 2 repls open at once

[chrisdickinson]

Looking into this. I see two possible solutions. If anyone has a better idea, please let me know!

1. Use tmp files + rename to atomically swap the node history file.
2. Use fcntl / flock'ing (or a similar mechanism) and make sure that only one cooperating node process at a time is writing to the file.

[bnoordhuis]

Option 1 can work as long as the files are on the same mount. Option 2 has portability issues and is unreliable with NFS.

[rvagg]

can we see what bash does and copy that?

[chrisdickinson]

What about mmap'ing the file & setting a semaphore byte? This might be a bit too heavyweight of a solution, since it would require mmap support be added to libuv.

[ivan]

Just for the record: this happens so frequently not because of racing writes but because both REPLs have the history file open in 'w' mode, one REPL exits and writes some history, and the other REPL exits and writes a shorter history, leaving behind some trailing bytes from the longer history.

[evanlucas]

It seems like since we moved away from using json, this is no longer prevalent. Do we still need to implement some sort of locking to prevent corruption?

[Fishrock123]

Locking would probably be desirable if someone wants to take it up. It would be pretty complex though.

[lance]

I started working on a locking implementation for this, but I'm not totally clear on the desired behavior. If multiple REPLs are open should history be interleaved? Or is it last-write-wins behavior?

[Fishrock123]

@lance either or. I'd prefer interleaved. Most terminals do last-write-is-latest-history.

[lance]

Removed good-first-contribution label as I don't think this is the case. At least not if #7005 is any indication.

[Trott]

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.