feat(@embark/core): store IPC files in a dir within os.tmpdir() #1214
Conversation
src/lib/core/ipc.js
Outdated
| this.ipcRole = options.ipcRole; | ||
| ipc.config.silent = true; | ||
| this.connected = false; | ||
| } | ||
|
|
||
| static ipcPath(basename, usePipePathOnWindows = false) { |
There was a problem hiding this comment.
Personally, I would prefer to see basename and usePipePathOnWindows passed in to the constructor, and have ipcPath as a get property of the IPC class with a backing variable, so that this does not have to get calculated every time it is called.
This means that line 82 of gethClient.js would change to:
cmd.push(`--ipcpath=${new IPC({basename: 'geth.ipc', usePipePathOnWindows: true}).ipcPath}`);
and line 56 in IPC.js would change to:
fs.mkdirpSync(utils.dirname(this.ipcPath));
Although having to new an instance for simple property get does seem a bit wasteful. I guess this is the sort of thing that Dependency Injection is good at solving...
There was a problem hiding this comment.
Originally, I wanted to put ipcPath() as a helper in core/fs.js, as a sibling of fs.dappPath() and fs.embarkPath(). But I ran into a subtle circular dependency problem, which was also the motivation for removing the utils/utils.js dependency in core/env.js. It may not be a problem anymore since utils/host.ts no longer needs to make use of dappPath (via anchoredPath() in core/env.js), but I'm not 100% sure.
I'm trying to think if there would be any problems instantiating IPC in those additional places.
There was a problem hiding this comment.
@emizzle thanks for helping me rethink this a bit! The circular dep problem is gone after dropping isDappMountedFromWindowsDockerHost (forced pushed into oblivion a couple of commits ago).
I've now decreased the surface area of this PR, removing the changes to core/env.js and relocating the ipcPath() helper to core/fs.js, which seems a better place.
Thoughts?
michaelsbradleyjr
force-pushed
the
refactor/ipc-location
branch
2 times, most recently
from
0eb210d
to
4bab76f
Compare
michaelsbradleyjr
force-pushed
the
refactor/ipc-location
branch
from
4bab76f
to
d1c615a
Compare
This PR replaces #1202. When embark is running on Linux and macOS, unix socket files are used for geth's and embark's IPC files. A problem can arise if a DApp is in a deeply nested directory structure such that character-length of the path to a socket file exceeds the maximum supported length. See #450. Also, if the DApp context is a Linux container running on a Windows Docker host, and if the DApp is mounted from the host's file system, there is a problem: unix socket files are incompatible with the Windows file system. Solve both problems at once by storing a DApp's `.ipc` files in a directory within `os.tmpdir()`. Introduce `ipcPath()` in `core/fs.js` and use a truncated SHA-512 hash of the DApp's path in the name of the temporary directory created for that purpose so that DApps won't collide (with an acceptably low probability of collision).
michaelsbradleyjr
force-pushed
the
refactor/ipc-location
branch
from
d1c615a
to
3670471
Compare
michaelsbradleyjr
changed the title
This PR replaces #1202.
When embark is running on Linux and macOS, unix socket files are used for geth's and embark's IPC files. A problem can arise if a DApp is in a deeply nested directory structure such that character-length of the path to a socket file exceeds the maximum supported length. See #450.
Also, if the DApp context is a Linux container running on a Windows Docker host, and if the DApp is mounted from the host's file system, there is a problem: unix socket files are incompatible with the Windows file system.
Solve both problems at once by storing a DApp's
.ipcfiles in a directory withinos.tmpdir(). IntroduceipcPath()incore/fs.jsand use a truncated SHA-512 hash of the DApp's path in the name of the temporary directory created for that purpose so that DApps won't collide (with an acceptably low probability of collision).