From patchwork Wed May 25 15:00:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 12861330 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 779AFC433F5 for ; Wed, 25 May 2022 15:02:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234064AbiEYPCJ (ORCPT ); Wed, 25 May 2022 11:02:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39896 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244959AbiEYPB2 (ORCPT ); Wed, 25 May 2022 11:01:28 -0400 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 56960AEE0E for ; Wed, 25 May 2022 08:01:24 -0700 (PDT) Received: by mail-wr1-x42e.google.com with SMTP id p10so11874924wrg.12 for ; Wed, 25 May 2022 08:01:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=pg+pIfJCewJfl3Uu7fv/1CMTNHr8lfgpM21Z9LQ/57U=; b=nXtd1+c5LGlcv3ilEAuHJPhomP96RzovGDM0kIVJgZlO84dpKUKkkE95YGkJvaTyOf WZvOkdRz8U+/O3rgLFyHlRVx4AcqaneM7XNYLw+uY1PpdApIjiQcFcYqI3Gt7HeVwJq2 Kirjw+M4QpBw3BCL23zDxQctQO3qsgZSzTZhBFMgasE9hPIKgSqnB0zXLKiIBdDbkhNx O0kjt37cFXWENiM0sgoY2C/OzRfbi0AYWiv849gZ2Zvi9a2KfCi+vIvRD4d5yJ9N8PUP 60qAZmyVrJOz11S0jDuER4vAbrZJ1/4nKd6fGqX5v9V3lwzaNiOUXTsx6PgqXCXmk6e9 gR0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=pg+pIfJCewJfl3Uu7fv/1CMTNHr8lfgpM21Z9LQ/57U=; b=BQEjzM7QswoJLprHtzMJEpudjN3lu6bz1H6lOxPLd5JpVR9bg6zXVs9QIv2OgiDAR3 dJSRNl5MRqOyL6ulzPHD2JGCWnxNm7BpyKBctAGfOu+CAKIfkEd60N5IPtOEK4D0QypQ FtMo7lOufWutiCEs1c0PtqUXwhCQnDhUwK2bNRlrgAmStZSN2ZZIArnjL7eyvHcjwAwc wwT058oubDvXQAYF3mZA9wO7AR/bkU8AE/dYCF0/krOnPNPxx6Sp6VfsvyVEjafLagRr Xp4aVAF024cmOTBKB4MagypL14hAKB1d78Q6qG/iHmhqNlSz2Bn7ZVQpneKgOvyuRMH9 IKXw== X-Gm-Message-State: AOAM533A7FYz3BycK5FWbQjJzFaZ9nv9c8O2rp5hAfxzBrmBOEgnyWJE Mpg4t7OyWMi0RAFFvX5CW/JAS6TLdqs= X-Google-Smtp-Source: ABdhPJzbKrEoLlaVq7dDhK/YosBqoSM6ICNeme2vv/k3mlNBP8aQzYFFuj8SG4KieVcsQtPGIZ1Jng== X-Received: by 2002:a5d:584c:0:b0:20f:de2e:bd1a with SMTP id i12-20020a5d584c000000b0020fde2ebd1amr12996170wrf.670.1653490882423; Wed, 25 May 2022 08:01:22 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id f21-20020a7bcc15000000b003942a244f57sm1883147wmh.48.2022.05.25.08.01.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 May 2022 08:01:21 -0700 (PDT) Message-Id: <7cf1be5f8e2c644a5bce9eb4484de2f6c82af359.1653490853.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 25 May 2022 15:00:40 +0000 Subject: [PATCH v8 18/30] fsm-health-win32: force shutdown daemon if worktree root moves Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff Hostetler , Derrick Stolee , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7A=?= Bjarmason , Torsten =?unknown-8bit?q?B?= =?unknown-8bit?q?=C3=B6gershausen?= , rsbecker@nexbridge.com, Bagas Sanjaya , Johannes Schindelin , Jeff Hostetler , Jeff Hostetler Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jeff Hostetler From: Jeff Hostetler Force shutdown fsmonitor daemon if the worktree root directory is moved, renamed, or deleted. Use Windows low-level GetFileInformationByHandle() to get and compare the Windows system unique ID for the directory with a cached version when we started up. This lets us detect the case where someone renames the directory that we are watching and then creates a new directory with the original pathname. This is important because we are listening to a named pipe for requests and they are stored in the Named Pipe File System (NPFS) which a kernel-resident pseudo filesystem not associated with the actual NTFS directory. For example, if the daemon was watching "~/foo/", it would have a directory-watch handle on that directory and a named-pipe handle for "//./pipe/...foo". Moving the directory to "~/bar/" does not invalidate the directory handle. (So the daemon would actually be watching "~/bar" but listening on "//./pipe/...foo". If the user then does "git init ~/foo" and causes another daemon to start, the first daemon will still have ownership of the pipe and the second daemon instance will fail to start. "git status" clients in "~/foo" will ask "//./pipe/...foo" about changes and the first daemon instance will tell them about "~/bar". This commit causes the first daemon to shutdown if the system unique ID for "~/foo" changes (changes from what it was when the daemon started). Shutdown occurs after a periodic poll. After the first daemon exits and releases the lock on the named pipe, subsequent Git commands may cause another daemon to be started on "~/foo". Similarly, a subsequent Git command may cause another daemon to be started on "~/bar". Signed-off-by: Jeff Hostetler --- compat/fsmonitor/fsm-health-win32.c | 143 ++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/compat/fsmonitor/fsm-health-win32.c b/compat/fsmonitor/fsm-health-win32.c index 24fc612bf02..2ea08c1d4e8 100644 --- a/compat/fsmonitor/fsm-health-win32.c +++ b/compat/fsmonitor/fsm-health-win32.c @@ -29,8 +29,150 @@ struct fsm_health_data HANDLE hHandles[1]; /* the array does not own these handles */ #define HEALTH_SHUTDOWN 0 int nr_handles; /* number of active event handles */ + + struct wt_moved + { + wchar_t wpath[MAX_PATH + 1]; + BY_HANDLE_FILE_INFORMATION bhfi; + } wt_moved; }; +/* + * Lookup the system unique ID for the path. This is as close as + * we get to an inode number, but this also contains volume info, + * so it is a little stronger. + */ +static int lookup_bhfi(wchar_t *wpath, + BY_HANDLE_FILE_INFORMATION *bhfi) +{ + DWORD desired_access = FILE_LIST_DIRECTORY; + DWORD share_mode = + FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE; + HANDLE hDir; + + hDir = CreateFileW(wpath, desired_access, share_mode, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hDir == INVALID_HANDLE_VALUE) { + error(_("[GLE %ld] health thread could not open '%ls'"), + GetLastError(), wpath); + return -1; + } + + if (!GetFileInformationByHandle(hDir, bhfi)) { + error(_("[GLE %ld] health thread getting BHFI for '%ls'"), + GetLastError(), wpath); + CloseHandle(hDir); + return -1; + } + + CloseHandle(hDir); + return 0; +} + +/* + * Compare the relevant fields from two system unique IDs. + * We use this to see if two different handles to the same + * path actually refer to the same *instance* of the file + * or directory. + */ +static int bhfi_eq(const BY_HANDLE_FILE_INFORMATION *bhfi_1, + const BY_HANDLE_FILE_INFORMATION *bhfi_2) +{ + return (bhfi_1->dwVolumeSerialNumber == bhfi_2->dwVolumeSerialNumber && + bhfi_1->nFileIndexHigh == bhfi_2->nFileIndexHigh && + bhfi_1->nFileIndexLow == bhfi_2->nFileIndexLow); +} + +/* + * Shutdown if the original worktree root directory been deleted, + * moved, or renamed? + * + * Since the main thread did a "chdir(getenv($HOME))" and our CWD + * is not in the worktree root directory and because the listener + * thread added FILE_SHARE_DELETE to the watch handle, it is possible + * for the root directory to be moved or deleted while we are still + * watching it. We want to detect that here and force a shutdown. + * + * Granted, a delete MAY cause some operations to fail, such as + * GetOverlappedResult(), but it is not guaranteed. And because + * ReadDirectoryChangesW() only reports on changes *WITHIN* the + * directory, not changes *ON* the directory, our watch will not + * receive a delete event for it. + * + * A move/rename of the worktree root will also not generate an event. + * And since the listener thread already has an open handle, it may + * continue to receive events for events within the directory. + * However, the pathname of the named-pipe was constructed using the + * original location of the worktree root. (Remember named-pipes are + * stored in the NPFS and not in the actual file system.) Clients + * trying to talk to the worktree after the move/rename will not + * reach our daemon process, since we're still listening on the + * pipe with original path. + * + * Furthermore, if the user does something like: + * + * $ mv repo repo.old + * $ git init repo + * + * A new daemon cannot be started in the new instance of "repo" + * because the named-pipe is still being used by the daemon on + * the original instance. + * + * So, detect move/rename/delete and shutdown. This should also + * handle unsafe drive removal. + * + * We use the file system unique ID to distinguish the original + * directory instance from a new instance and force a shutdown + * if the unique ID changes. + * + * Since a worktree move/rename/delete/unmount doesn't happen + * that often (and we can't get an immediate event anyway), we + * use a timeout and periodically poll it. + */ +static int has_worktree_moved(struct fsmonitor_daemon_state *state, + enum interval_fn_ctx ctx) +{ + struct fsm_health_data *data = state->health_data; + BY_HANDLE_FILE_INFORMATION bhfi; + int r; + + switch (ctx) { + case CTX_TERM: + return 0; + + case CTX_INIT: + if (xutftowcs_path(data->wt_moved.wpath, + state->path_worktree_watch.buf) < 0) { + error(_("could not convert to wide characters: '%s'"), + state->path_worktree_watch.buf); + return -1; + } + + /* + * On the first call we lookup the unique sequence ID for + * the worktree root directory. + */ + return lookup_bhfi(data->wt_moved.wpath, &data->wt_moved.bhfi); + + case CTX_TIMER: + r = lookup_bhfi(data->wt_moved.wpath, &bhfi); + if (r) + return r; + if (!bhfi_eq(&data->wt_moved.bhfi, &bhfi)) { + error(_("BHFI changed '%ls'"), data->wt_moved.wpath); + return -1; + } + return 0; + + default: + die(_("unhandled case in 'has_worktree_moved': %d"), + (int)ctx); + } + + return 0; +} + + int fsm_health__ctor(struct fsmonitor_daemon_state *state) { struct fsm_health_data *data; @@ -64,6 +206,7 @@ void fsm_health__dtor(struct fsmonitor_daemon_state *state) * A table of the polling functions. */ static interval_fn *table[] = { + has_worktree_moved, NULL, /* must be last */ };