From a885980e86627e0e4408ef8aa7179cbdd67d3295 Mon Sep 17 00:00:00 2001 From: James Foster Date: Mon, 12 Oct 2026 17:17:20 -0300 Subject: [PATCH] Apply RemoveIPC=no at once, and report it only once it is in effect setRemoveIPC.sh wrote logind's config and stopped, and the "Survives logout" row read that file, so the row vanished while the running logind still had RemoveIPC=yes -- and logind reads the loaded value when a user's last session ends, so the next logout killed the database anyway. The script now sends logind SIGHUP, which re-reads its config without a restart, and waits for the live value to change. The row reads logind's RemoveIPC D-Bus property (any user may) and falls back to the files when busctl or logind cannot answer; a file logind has not loaded shows as "not yet applied" instead of hiding the row. unset-os-config.sh reloads the same way. Measured on systemd 249 (Ubuntu 22.04) and 255 (24.04) in containers: writing the file alone leaves the property true; the script flips it with no logind restart; a test user's shared-memory segment survives logout after the script and is destroyed under the default. `--kill-whom` is not used because systemd 249's systemctl does not know it. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 +- docs/automation-boundary.md | 8 ++- resources/setRemoveIPC.sh | 21 ++++++- scripts/unset-os-config.sh | 8 ++- src/__tests__/activation.test.ts | 3 +- src/__tests__/osConfig.test.ts | 43 ++++++++++++- src/osConfig.ts | 101 ++++++++++++++++++++++++++++--- src/statusView.ts | 21 +++++-- 8 files changed, 185 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index d83a333..6358cfa 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,8 @@ machine-wide, it prompts you and waits for your permission: the shared-memory dialog appears, it lists this change as recommended, and GemDB Code runs it in its own terminal. Otherwise, click the **Survives logout** row in the GemDB Code sidebar, or run **GemDB: Keep the Database Running After Logout** from the Command Palette. The change takes - effect after you restart your computer. + effect at once. If systemd cannot load it straight away, the row stays and reads **not yet + applied** until you restart your computer. #### Your shell profile and AI clients diff --git a/docs/automation-boundary.md b/docs/automation-boundary.md index 8d6fb3b..3a598d4 100644 --- a/docs/automation-boundary.md +++ b/docs/automation-boundary.md @@ -158,7 +158,13 @@ modal when that is being asked anyway. Otherwise the status view's "Survives logout" row offers it (`gemdb.configureRemoveIpc`). It used to raise the modal alone, which on stock Linux (where shared memory is already far above 1 GB) meant every Linux user was asked for `sudo` over a setting that blocks -nothing (issue #45). +nothing (issue #45). The script sends logind SIGHUP so the setting applies +without a reboot, and the row reads logind's live `RemoveIPC` property rather +than its config files: logind consults the loaded value when a user's last +session ends, so a file it has not reloaded protects nothing, and checking +the file let the row vanish while a logout still killed the database. +Measured 2026-10-12 on systemd 249 and 255: the reload applies it, no restart +of logind, and a segment survives a logout that destroys it under the default. **Choosing another root path when the default is on NFS — asked, once setup has refused.** The stone will not open a repository on an NFS mount, and on diff --git a/resources/setRemoveIPC.sh b/resources/setRemoveIPC.sh index 0f2da2c..8c133f3 100755 --- a/resources/setRemoveIPC.sh +++ b/resources/setRemoveIPC.sh @@ -22,4 +22,23 @@ RemoveIPC=no EOF echo "Configured at $CONF_FILE" -echo "To apply: restart your computer, or run: sudo systemctl restart systemd-logind" + +# Writing the file is not enough: the running logind keeps the value it last +# loaded, and it reads that value when a user's last session ends -- so until +# it reloads, a logout still destroys the database. logind re-reads its config +# on SIGHUP (every systemd since at least v245), which applies the change now +# without the session-disrupting restart. `systemctl kill` rather than +# `reload` because older units define no ExecReload. +if systemctl kill --signal=SIGHUP systemd-logind.service 2>/dev/null; then + # The reload is asynchronous; wait briefly for the running value to change. + for _ in 1 2 3 4 5 6 7 8 9 10; do + if [ "$(busctl get-property org.freedesktop.login1 /org/freedesktop/login1 \ + org.freedesktop.login1.Manager RemoveIPC 2>/dev/null)" = "b false" ]; then + echo "Applied: systemd-logind now keeps IPC at logout." + exit 0 + fi + sleep 0.2 + done +fi + +echo "systemd-logind has not loaded the change. Restart your computer to apply it." diff --git a/scripts/unset-os-config.sh b/scripts/unset-os-config.sh index c5b3cfa..2cae6c5 100755 --- a/scripts/unset-os-config.sh +++ b/scripts/unset-os-config.sh @@ -84,8 +84,12 @@ Linux) if [ -f "${IPC_CONF}" ]; then rm -f "${IPC_CONF}" echo "Removed ${IPC_CONF}" - echo " RemoveIPC goes back to the systemd default at the next boot, or now" - echo " with: sudo systemctl restart systemd-logind" + # logind re-reads its config on SIGHUP, as setRemoveIPC.sh relies on. + if systemctl kill --signal=SIGHUP systemd-logind.service 2>/dev/null; then + echo " systemd-logind reloaded: RemoveIPC is back to the systemd default" + else + echo " RemoveIPC goes back to the systemd default at the next boot" + fi fi # The kernel's own default since 3.16: ULONG_MAX - (1 << 24), i.e. no diff --git a/src/__tests__/activation.test.ts b/src/__tests__/activation.test.ts index eaa69c8..c170baf 100644 --- a/src/__tests__/activation.test.ts +++ b/src/__tests__/activation.test.ts @@ -54,11 +54,12 @@ const ensureOsConfigured = vi.fn(async (): Promise => 'alreadyConfigured const isSharedMemoryConfigured = vi.fn(async () => false); vi.mock('../osConfig', async (importOriginal) => ({ osConfigAllowsStart: (await importOriginal()).osConfigAllowsStart, + REMOVE_IPC_STATUS: (await importOriginal()).REMOVE_IPC_STATUS, configureSharedMemory: async () => {}, configureRemoveIpc: async () => {}, ensureOsConfigured: () => ensureOsConfigured(), isSharedMemoryConfigured: () => isSharedMemoryConfigured(), - isRemoveIpcConfigured: () => false, + removeIpcStatus: async () => 'unset', sharedMemoryLabel: async () => '', })); diff --git a/src/__tests__/osConfig.test.ts b/src/__tests__/osConfig.test.ts index 1817d85..733deea 100644 --- a/src/__tests__/osConfig.test.ts +++ b/src/__tests__/osConfig.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from 'vitest'; import { REQUIRED_SHARED_MEMORY_GB } from '../config'; -import { OsConfigWorld, runEnsureOsConfigured } from '../osConfig'; +import { + OsConfigWorld, + parseRemoveIpcProperty, + removeIpcStatusFrom, + runEnsureOsConfigured, +} from '../osConfig'; /** * A stand-in operating system that records what was asked of it. @@ -31,7 +36,8 @@ function makeWorld(options: { const world: OsConfigWorld = { sharedMemoryOk: () => Promise.resolve(sharedMemory === 'ok' || (sharedMemory === 'fixed' && sharedMemoryRan)), - removeIpcOk: () => removeIpc === 'ok' || (removeIpc === 'fixed' && removeIpcRan), + removeIpcOk: () => + Promise.resolve(removeIpc === 'ok' || (removeIpc === 'fixed' && removeIpcRan)), confirm: (message) => { calls.push('confirm'); messages.push(message); @@ -164,3 +170,36 @@ describe('runEnsureOsConfigured', () => { expect(recommended).toContain('RemoveIPC=no'); }); }); + +describe('removeIpcStatusFrom', () => { + it('is not in effect while only the config file has it', () => { + // The state the status view used to call fixed: the script wrote the + // file, logind never reloaded it, and a logout still killed the database. + expect(removeIpcStatusFrom(false, true)).toBe('pending'); + }); + + it('trusts the running logind over the config files', () => { + expect(removeIpcStatusFrom(true, false)).toBe('inEffect'); + expect(removeIpcStatusFrom(false, false)).toBe('unset'); + }); + + it('falls back to the config files when logind cannot be asked', () => { + expect(removeIpcStatusFrom(undefined, true)).toBe('inEffect'); + expect(removeIpcStatusFrom(undefined, false)).toBe('unset'); + }); +}); + +describe('parseRemoveIpcProperty', () => { + it('reads RemoveIPC=no as keeping IPC', () => { + expect(parseRemoveIpcProperty('b false\n')).toBe(true); + }); + + it('reads RemoveIPC=yes as removing IPC', () => { + expect(parseRemoveIpcProperty('b true\n')).toBe(false); + }); + + it('has no answer for output that is not a boolean property', () => { + expect(parseRemoveIpcProperty('')).toBeUndefined(); + expect(parseRemoveIpcProperty('s "no"')).toBeUndefined(); + }); +}); diff --git a/src/osConfig.ts b/src/osConfig.ts index 99eb158..55e7c3f 100644 --- a/src/osConfig.ts +++ b/src/osConfig.ts @@ -113,6 +113,84 @@ export async function sharedMemoryLabel(): Promise { return smallest >= 1024 ? '≥ 1 TB' : `${Math.round(smallest * 1000) / 1000} GB`; } +/** + * Where `RemoveIPC=no` stands, as far as a logout is concerned. + * + * inEffect — the running logind has it, so a logout leaves the database alone. + * pending — it is written to logind's config, but the running logind has not + * read it, so a logout still destroys the database. Without this + * state the status view called a machine fixed the moment the + * script wrote the file, and the next logout killed the database + * anyway. + * unset — neither. + */ +export const REMOVE_IPC_STATUS = { + inEffect: 'inEffect', + pending: 'pending', + unset: 'unset', +} as const; +export type RemoveIpcStatus = (typeof REMOVE_IPC_STATUS)[keyof typeof REMOVE_IPC_STATUS]; + +/** + * Combine what the running logind says with what its config files say. + * + * The running value is the truth, because logind reads `remove_ipc` at the + * moment a user's last session ends (`user_finalize`). It is undefined when it + * cannot be read — no `busctl`, or no logind on the bus — and then the config + * files are the best guess left, which is all this check used to read. + */ +export function removeIpcStatusFrom( + running: boolean | undefined, + inConfig: boolean, +): RemoveIpcStatus { + if (running === undefined) return inConfig ? REMOVE_IPC_STATUS.inEffect : REMOVE_IPC_STATUS.unset; + if (running) return REMOVE_IPC_STATUS.inEffect; + return inConfig ? REMOVE_IPC_STATUS.pending : REMOVE_IPC_STATUS.unset; +} + +/** + * Read `busctl get-property`'s answer for logind's `RemoveIPC`: true when IPC + * is kept (`RemoveIPC=no`), undefined when the answer is not a boolean. + */ +export function parseRemoveIpcProperty(stdout: string): boolean | undefined { + const match = stdout.trim().match(/^b\s+(true|false)$/); + return match ? match[1] === 'false' : undefined; +} + +/** + * Does the running logind keep IPC at logout? Read from its `RemoveIPC` + * D-Bus property, which any user may read — it reflects the config logind + * last loaded, so a file written since then does not count until logind is + * sent SIGHUP (which `setRemoveIPC.sh` does) or restarted. + */ +function isRemoveIpcRunning(): Promise { + return new Promise((resolve) => { + execFile( + 'busctl', + [ + 'get-property', + 'org.freedesktop.login1', + '/org/freedesktop/login1', + 'org.freedesktop.login1.Manager', + 'RemoveIPC', + ], + { encoding: 'utf-8', timeout: 5000 }, + (error, stdout) => resolve(error ? undefined : parseRemoveIpcProperty(stdout)), + ); + }); +} + +/** Where `RemoveIPC=no` stands on this machine; always `inEffect` off Linux. */ +export async function removeIpcStatus(): Promise { + if (process.platform !== 'linux') return REMOVE_IPC_STATUS.inEffect; + return removeIpcStatusFrom(await isRemoveIpcRunning(), isRemoveIpcInConfig()); +} + +/** Will the database survive its owner logging out? Always true off Linux. */ +export async function isRemoveIpcConfigured(): Promise { + return (await removeIpcStatus()) === REMOVE_IPC_STATUS.inEffect; +} + /** * Does systemd's logind config set `RemoveIPC=no`? * @@ -120,9 +198,7 @@ export async function sharedMemoryLabel(): Promise { * main file, and the last assignment wins — so read them in that order and * keep the final value rather than stopping at the first hit. */ -export function isRemoveIpcConfigured(): boolean { - if (process.platform !== 'linux') return true; - +function isRemoveIpcInConfig(): boolean { const dropInDir = '/etc/systemd/logind.conf.d'; let dropIns: string[] = []; try { @@ -164,7 +240,7 @@ export function isRemoveIpcConfigured(): boolean { */ export interface OsConfigWorld { sharedMemoryOk: () => Promise; - removeIpcOk: () => boolean; + removeIpcOk: () => Promise; /** Show the modal, already worded. True if the user chose to configure. */ confirm: (message: string) => Promise; runSharedMemoryScript: () => Promise; @@ -201,7 +277,7 @@ export async function runEnsureOsConfigured(world: OsConfigWorld): Promise { - const wasUnset = !isRemoveIpcConfigured(); + const wasUnset = !(await isRemoveIpcConfigured()); await runSetupScript( REMOVE_IPC_TERMINAL, path.join(extensionPath, 'resources', 'setRemoveIPC.sh'), ); - const configured = isRemoveIpcConfigured(); + const status = await removeIpcStatus(); + const configured = status === REMOVE_IPC_STATUS.inEffect; if (wasUnset) { reportOsConfigPrompted( TRIGGER.removeIpcCommand, @@ -354,8 +431,12 @@ export async function configureRemoveIpc(extensionPath: string): Promise { } if (configured) { void vscode.window.showInformationMessage( - 'The database will now keep running when you log out. ' + - 'This takes effect after you restart your computer.', + 'The database will now keep running when you log out.', + ); + } else if (status === REMOVE_IPC_STATUS.pending) { + void vscode.window.showWarningMessage( + 'RemoveIPC=no is saved, but systemd has not loaded it yet, so the database will still ' + + 'stop if you log out. Restart your computer to apply it.', ); } else { void vscode.window.showErrorMessage( diff --git a/src/statusView.ts b/src/statusView.ts index 986584c..be2ef1f 100644 --- a/src/statusView.ts +++ b/src/statusView.ts @@ -26,7 +26,12 @@ import { spaceReading, usedMb, } from './maintenance'; -import { isRemoveIpcConfigured, isSharedMemoryConfigured, sharedMemoryLabel } from './osConfig'; +import { + REMOVE_IPC_STATUS, + isSharedMemoryConfigured, + removeIpcStatus, + sharedMemoryLabel, +} from './osConfig'; import { databaseExists, databasePath, @@ -570,14 +575,20 @@ export class StatusViewProvider implements vscode.TreeDataProvider { }); // Only worth a row when it is wrong — on macOS it never applies, and on a - // correctly configured Linux box it is noise. - if (process.platform === 'linux' && !isRemoveIpcConfigured()) { + // correctly configured Linux box it is noise. "Wrong" means the running + // logind would still remove IPC, not just that the config file says so. + const removeIpc = await removeIpcStatus(); + if (removeIpc !== REMOVE_IPC_STATUS.inEffect) { + const pending = removeIpc === REMOVE_IPC_STATUS.pending; rows.push({ label: 'Survives logout', - description: 'not configured', + description: pending ? 'not yet applied' : 'not configured', tooltip: 'systemd will destroy the database’s shared memory when you log out of this machine. ' + - 'Click to fix it — this runs a setup script with sudo.', + (pending + ? 'The fix is saved but systemd has not loaded it. Click to apply it now, or ' + + 'restart your computer.' + : 'Click to fix it — this runs a setup script with sudo.'), icon: warn('warning'), command: { command: 'gemdb.configureRemoveIpc',