Skip to content

Commit 059e81a

Browse files
author
andy
committed
Fix RCommand freeze/error on local ("system") connections (regression from bash-support change)
Reported live within minutes of shipping v8.0.1: ECCE froze/errored ("ERROR: Unable to create local command shell.") on Save in the calculation editor. Root cause: the tcsh/bash detection probe added for remote connections in the previous commit runs unconditionally in RCommand's constructor -- but that same login sequence also handles RCommand("system")/"-f", the *local*-machine connection ESInputController.C/JobStore.C/Launch.C use to replace plain system() calls (a locally spawned shell talking over a pty, same handshake mechanism as a real remote connection). The new probe was never meant to run there at all -- Point 1 (the local shell) was explicitly scoped out as separate, not-yet-started work when Point 2 (remote) was designed. Fixed by skipping the whole detection block (probe, forced-bash checks) for local connections, forcing useBash=false unconditionally -- exactly the pre-existing behavior before bash support existed. Verified via a standalone repro linked against the real build (RCommand("system") + isOpen(), the exact call ESInputController.C's save path makes): hung/ failed before this fix, returns true immediately after.
1 parent 9dab8e2 commit 059e81a

1 file changed

Lines changed: 22 additions & 3 deletions

File tree

‎src/comm/rcommand/RCommand.C‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,14 +1271,33 @@ hopToIt:
12711271
// after this). Uses expect2() rather than expect1() because we don't
12721272
// have a reliable prompt pattern to anchor on yet -- that's the whole
12731273
// point of the sentinel-prompt command this probe runs before.
1274+
// This whole login sequence -- including the block below -- also runs
1275+
// for RCommand("system")/"-f", the *local*-machine connection used to
1276+
// replace plain system() calls (ESInputController.C, JobStore.C,
1277+
// Launch.C, ...), not just genuine remote connections: it's a locally
1278+
// spawned shell talking over a pty, same handshake either way. That's
1279+
// pre-existing, unrelated to this change. But it means the probe below
1280+
// -- meant only for the "remote account's login shell might be bash"
1281+
// problem (Point 2) -- must NOT run for local connections at all: this
1282+
// was reported live as a hang/error ("Unable to create local command
1283+
// shell") the very first time this shipped, exactly here. Point 1 (the
1284+
// local shell) is explicitly a separate, not-yet-started piece of work
1285+
// (see RCommand::shellCommand()'s locShell-based -fc/-f/-i flag
1286+
// construction) -- until that's done deliberately, local connections
1287+
// must behave exactly as they did before this whole feature existed.
1288+
bool isLocalConnection = (machine == "system" || machine == "-f");
12741289
bool useBash = false;
1275-
bool forcedBash = (getenv("ECCE_FORCE_BASH_SHELL") != 0);
1276-
if (!forcedBash) {
1290+
bool forcedBash = false;
1291+
if (!isLocalConnection)
1292+
forcedBash = (getenv("ECCE_FORCE_BASH_SHELL") != 0);
1293+
if (!isLocalConnection && !forcedBash) {
12771294
RefMachine* forceCheckMachine = RefMachine::refLookup(theMachine);
12781295
if (forceCheckMachine && forceCheckMachine->forceBashShell())
12791296
forcedBash = true;
12801297
}
1281-
if (forcedBash) {
1298+
if (isLocalConnection) {
1299+
useBash = false;
1300+
} else if (forcedBash) {
12821301
useBash = true;
12831302
} else {
12841303
if (!expwrite("tcsh -c 'echo ECCE_HAS_TCSH' || echo ECCE_NO_TCSH"))

0 commit comments

Comments
 (0)