From: Jeff Hostetler Date: Wed, 10 Feb 2021 21:31:23 GMT Subject: Re: [PATCH v2 11/14] unix-socket: add options to unix_stream_listen() Message-ID: In-Reply-To: On 2/10/21 10:55 AM, Jeff King wrote: > On Tue, Feb 09, 2021 at 12:39:22PM -0500, Jeff Hostetler wrote: > >> Yeah, in my version two processes could still create uniquely named >> sockets and then do the rename trick. But they capture the inode >> number of the socket before they do that. They periodically lstat >> the socket to see if the inode number has changed and if so, assume >> it has been stolen from them. (A bit of a hack, I admit.) > > OK, that makes more sense. I saw the mention of the inode stuff in a > comment, but I didn't see it in the code (I guess if it's a periodic > check it's not in that initial socket creation function). > Yeah, there's a very slow poll(2) loop in the listen/accept thread that watches for that and new connections (and quit messages). >> And I was assuming that 2 servers starting at about the same time >> are effectively equivalent -- it doesn't matter which one dies, since >> they both should have the same amount of cached state. Unlike the >> case where a long-running server (with lots of state) is replaced by >> a newcomer. > > Yeah, I agree with that notion in general. I do think it would be easier > to reason about if the creation were truly race-proof (probably with a > dot-lock; see below), rather than the later "check if we got replaced" > thing. OTOH, that "check" strategy covers a variety of cases (including > that somebody tried to ping us, decided we weren't alive due to a > timeout or some other system reason, and then replaced our socket). > > Another strategy there could be having the daemon just decide to quit if > nobody contacts it for N time units. It is, after all, a cache. Even if > nobody replaces the socket, it probably makes sense to eventually decide > that the memory we're holding isn't going to good use. I have the poll(2) loop set to recheck the inode for theft every 60 seconds (randomly chosen). Assuming the socket isn't stolen, I want to leave any thoughts of an auto-shutdown to the application layer above it. My next patch series will use this ipc mechanism to build a FSMonitor daemon that will watch the filesystem for changes and then be able to quickly respond to a `git status`, so it is important that it be allowed to run without any clients for a while (such a during a build). Yes, memory concerns are important, so I do want it to auto-shutdown if the socket is stolen (or the workdir is deleted). > >> I am wondering if we can use the .LOCK file magic to our advantage >> here (in sort of an off-label use). If we have the server create a >> lockfile ".LOCK" and if successful leave it open/locked for the >> life of the server (rather than immediately renaming it onto ) >> and let the normal shutdown code rollback/delete the lockfile in the >> cleanup/atexit. >> >> If the server successfully creates the lockfile, then unlink+create >> the socket at . > > I don't even think this is off-label. Though the normal use is for the > .lock file to get renamed into place as the official file, there are a > few other places where we use it solely for mutual exclusion. You just > always end with rollback_lock_file(), and never "commit" it. > > So something like: > > 1. Optimistically see if socket "foo" is present and accepting > connections. > > 2. If not, then take "foo.lock". If somebody else is holding it, loop > with a timeout waiting for them to come alive. > > 3. Assuming we got the lock, then either unlink+create the socket as > "foo", or rename-into-place. I don't think it matters that much > which. > > 4. Rollback "foo.lock", unlinking it. > > Then one process wins the lock and creates the socket, while any > simultaneous creators spin in step 2, and eventually connect to the > winner. > >> That would give us the unique/exclusive creation (on the lock) that >> we need. Then wrap that with all the edge case cleanup code to >> create/delete/manage the peer socket. Basically if the lock exists, >> there should be a live server listening to the socket (unless there >> was a crash...). > > I think you'd want to delete the lock as soon as you're done with the > setup. That reduces the chances that a dead server (e.g., killed by a > power outage without the chance to clean up after itself) leaves a stale > lock sitting around. Thanks this helps. I've got a version now that is a slight variation on what you have here that seems to work nicely and has the short-lived lock file. I'll post this shortly. Jeff