go-rod / rod

A Chrome DevTools Protocol driver for web automation and scraping.
https://go-rod.github.io
MIT License
5.22k stars 343 forks source link

Use lockfiles instead of LockPort #493

Open egonelbre opened 2 years ago

egonelbre commented 2 years ago

Currently I'm using go-rod in an environment where there are randomly chosen ports being used. So there's a slight chance that any given port will be used -- deadlocking the test since it cannot acquire the port.

A common approach for these would be a lockfile in the directory where you are downloading (e.g. https://github.com/gofrs/flock). There might be other problems with that approach that I'm not aware of.

An other workaround could be to allow setting a different localhost making it less likely to have conflicts for the port selection. e.g. 127.0.0.200:2978.

If one of the approaches seems reasonable, I can send a PR.

ysmood commented 2 years ago

Yes, agree, we could let unix like os use it and think about how to do it on Windows.

The actual code that is doing it in leakless project, would you like to work on this?

egonelbre commented 2 years ago

Sure, I can work on this. Also, I just realized that there could also be a nicer solution, that doesn't use lock files per se.

The download path could:

  1. check whether the binary exists, if it does, then use it
  2. if it doesn't, then try create a temporary file (that is autodeleted on when process closes) for downloading
  3. if the temporary file already exists, then poll the temporary file until it's deleted
  4. repeat from 1.

That way the temporary file being download could act as the lock.

ysmood commented 2 years ago

We have already used this way before, this way can't prevent multiple process of rod from racing. We want it to be safe on the same machine. What do you think?

egonelbre commented 2 years ago

Yes, that should work fine on the same machine and even on multiple docker images mounted the same cache. In principle it's still a file-lock, but it's using the file being downloaded file as the lock (note, the temporary file wouldn't be a random filename, but based on the target location). It could use the .zip file for itself.

ysmood commented 2 years ago

Let me try it, I think the open syscall for all modern OSes are thread-safe for the same path.