Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition on history file with multiple REPLs #1634

Closed
rvagg opened this issue May 6, 2015 · 11 comments
Closed

Race condition on history file with multiple REPLs #1634

rvagg opened this issue May 6, 2015 · 11 comments
Assignees
Labels
confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem.

Comments

@rvagg
Copy link
Member

rvagg commented May 6, 2015

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
@brendanashworth brendanashworth added the repl Issues and PRs related to the REPL subsystem. label May 6, 2015
@chrisdickinson
Copy link
Contributor

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.

@chrisdickinson chrisdickinson self-assigned this May 7, 2015
@bnoordhuis
Copy link
Member

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
Copy link
Member Author

rvagg commented May 7, 2015

can we see what bash does and copy that?

@chrisdickinson
Copy link
Contributor

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
Copy link
Contributor

ivan commented May 13, 2015

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.

@Fishrock123 Fishrock123 added the confirmed-bug Issues with confirmed bugs. label Aug 3, 2015
@evanlucas
Copy link
Contributor

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 Fishrock123 added good first issue Issues that are suitable for first-time contributors. mentor-available labels Mar 16, 2016
@Fishrock123
Copy link
Contributor

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

@lance
Copy link
Member

lance commented May 25, 2016

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
Copy link
Contributor

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

@lance lance removed the good first issue Issues that are suitable for first-time contributors. label Aug 3, 2016
@lance
Copy link
Member

lance commented Aug 3, 2016

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

@Trott
Copy link
Member

Trott commented Jul 9, 2017

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.

@Trott Trott closed this as completed Jul 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

9 participants