Merge "veyron/services/mgmt/node: code cleanup TODO"
diff --git a/lib/filelocker/locker_test.go b/lib/filelocker/locker_test.go
index d98e093..0060210 100644
--- a/lib/filelocker/locker_test.go
+++ b/lib/filelocker/locker_test.go
@@ -62,7 +62,7 @@
 	filepath := newFile()
 	defer os.Remove(filepath)
 
-	sh := modules.NewShell("testLockUnlockChild")
+	sh := modules.NewShell()
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 	h, err := sh.Start("testLockUnlockChild", nil, filepath)
 	if err != nil {
diff --git a/lib/modules/core/core.go b/lib/modules/core/core.go
index add670f..214d127 100644
--- a/lib/modules/core/core.go
+++ b/lib/modules/core/core.go
@@ -48,8 +48,6 @@
 //    runs a proxy server
 package core
 
-import "veyron.io/veyron/veyron/lib/modules"
-
 const (
 	// Functions
 	LSCommand                = "ls"
@@ -70,50 +68,3 @@
 	WSPRCommand        = "wsprd"
 	ShellCommand       = "sh"
 )
-
-// NewShell returns a new Shell instance with the core commands installed.
-func NewShell() *modules.Shell {
-	shell := modules.NewShell("")
-	Install(shell)
-	return shell
-}
-
-// Install installs the core commands into the supplied Shell.
-func Install(shell *modules.Shell) {
-	// Explicitly add the subprocesses so that we can provide a help string
-	shell.AddSubprocess(EchoServerCommand, `<message> <name>
-	mount an echo server at <name>, it will prepend <message> to its responses`)
-	shell.AddSubprocess(EchoClientCommand, `
-		<name> <text>
-		invoke name.Echo(<text>)`)
-	shell.AddSubprocess(RootMTCommand, `run a root mount table
-		it will output MT_NAME, MT_ADDR and PID`)
-	shell.AddSubprocess(MTCommand, `<name>
-		run a mount table mounted at <name>`)
-	shell.AddSubprocess(LSExternalCommand, `<glob>
-		run a glob command as an external subprocess`)
-	shell.AddSubprocess(ProxyServerCommand, `<name>...
-		run a proxy server mounted at the specified names`)
-	// TODO(sadovsky): It's unfortunate that we must duplicate help strings
-	// between RegisterChild and AddSubprocess. Will be fixed by my proposed
-	// refactoring.
-	shell.AddSubprocess(WSPRCommand, usageWSPR())
-	//shell.AddSubprocess(ShellCommand, subshell, "")
-
-	shell.AddFunction(LSCommand, ls, `<glob>...
-	issues glob requests using the current processes namespace library`)
-	shell.AddFunction(ResolveCommand, resolveObject, `<name>
-	resolves name to obtain an object server address`)
-	shell.AddFunction(ResolveMTCommand, resolveMT, `<name>
-	resolves name to obtain a mount table address`)
-	shell.AddFunction(SetNamespaceRootsCommand, setNamespaceRoots, `<name>...
-	set the in-process namespace roots to <name>...`)
-	shell.AddFunction(SleepCommand, sleep, `[duration]
-	sleep for a time (in go time.Duration format): defaults to 1s`)
-	shell.AddFunction(TimeCommand, now, `
-	prints the current time`)
-	shell.AddFunction(NamespaceCacheCommand, namespaceCache, `on|off
-	turns the namespace cache on or off`)
-	shell.AddFunction(MountCommand, mountServer, `<mountpoint> <server> <ttl> [M][R]
-	invokes namespace.Mount(<mountpoint>, <server>, <ttl>)`)
-}
diff --git a/lib/modules/core/core_test.go b/lib/modules/core/core_test.go
index 1cc69e2..5f72ee9 100644
--- a/lib/modules/core/core_test.go
+++ b/lib/modules/core/core_test.go
@@ -23,7 +23,7 @@
 )
 
 func TestCommands(t *testing.T) {
-	sh := core.NewShell()
+	sh := modules.NewShell()
 	defer sh.Cleanup(nil, os.Stderr)
 	for _, c := range []string{core.RootMTCommand, core.MTCommand} {
 		if len(sh.Help(c)) == 0 {
@@ -38,7 +38,7 @@
 }
 
 func newShell() (*modules.Shell, func()) {
-	sh := core.NewShell()
+	sh := modules.NewShell()
 	return sh, func() {
 		if testing.Verbose() {
 			vlog.Infof("------ cleanup ------")
diff --git a/lib/modules/core/misc.go b/lib/modules/core/misc.go
index 577f3b2..35b54c2 100644
--- a/lib/modules/core/misc.go
+++ b/lib/modules/core/misc.go
@@ -11,6 +11,19 @@
 	"veyron.io/veyron/veyron/lib/modules"
 )
 
+func init() {
+
+	modules.RegisterFunction(NamespaceCacheCommand, `on|off
+	turns the namespace cache on or off`, namespaceCache)
+	modules.RegisterFunction(MountCommand, `<mountpoint> <server> <ttl> [M][R]
+	invokes namespace.Mount(<mountpoint>, <server>, <ttl>)`, mountServer)
+	modules.RegisterFunction(SleepCommand, `[duration]
+	sleep for a time(in go time.Duration format): defaults to 1s`, sleep)
+	modules.RegisterFunction(TimeCommand, `
+	prints the current time`, now)
+
+}
+
 func sleep(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
 	d := time.Second
 	if len(args) > 1 {
diff --git a/lib/modules/core/mounttable.go b/lib/modules/core/mounttable.go
index 7d5d7f8..51373a6 100644
--- a/lib/modules/core/mounttable.go
+++ b/lib/modules/core/mounttable.go
@@ -22,6 +22,18 @@
 	modules.RegisterChild(LSExternalCommand, `<glob>...
 	issues glob requests using the current processes namespace library`,
 		ls)
+	modules.RegisterFunction(LSCommand, `<glob>...
+	issues glob requests using the current processes namespace library`, ls)
+	modules.RegisterFunction(ResolveCommand, `<name>
+	resolves name to obtain an object server address`, resolveObject)
+	modules.RegisterFunction(ResolveMTCommand, `<name>
+	resolves name to obtain a mount table address`, resolveMT)
+	modules.RegisterFunction(SetNamespaceRootsCommand, `<name>...
+	set the in-process namespace roots to <name>...`, setNamespaceRoots)
+	modules.RegisterFunction(NamespaceCacheCommand, `on|off
+	turns the namespace cache on or off`, namespaceCache)
+	modules.RegisterFunction(MountCommand, `<mountpoint> <server> <ttl> [M][R]
+	invokes namespace.Mount(<mountpoint>, <server>, <ttl>)`, mountServer)
 }
 
 func mountTable(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
diff --git a/lib/modules/exec.go b/lib/modules/exec.go
index 6d45f6c..d656d0a 100644
--- a/lib/modules/exec.go
+++ b/lib/modules/exec.go
@@ -2,11 +2,9 @@
 
 import (
 	"flag"
-	"fmt"
 	"io"
 	"os"
 	"os/exec"
-	"regexp"
 	"strings"
 	"sync"
 	"time"
@@ -63,14 +61,8 @@
 	return runFlag.Value.String() == "TestHelperProcess"
 }
 
-// IsModulesProcess returns true if this process is run using
-// the modules package.
-func IsModulesProcess() bool {
-	return os.Getenv(ShellEntryPoint) != ""
-}
-
 func newExecHandle(name string) command {
-	return &execHandle{name: name, entryPoint: ShellEntryPoint + "=" + name}
+	return &execHandle{name: name, entryPoint: shellEntryPoint + "=" + name}
 }
 
 func (eh *execHandle) Stdout() io.Reader {
@@ -101,11 +93,11 @@
 	newargs := []string{os.Args[0]}
 	newargs = append(newargs, testFlags()...)
 	newargs = append(newargs, args...)
-	// Be careful to remove any existing ShellEntryPoint env vars. This
+	// Be careful to remove any existing shellEntryPoint env vars. This
 	// can happen when subprocesses run other subprocesses etc.
 	cleaned := make([]string, 0, len(env)+1)
 	for _, e := range env {
-		if strings.HasPrefix(e, ShellEntryPoint+"=") {
+		if strings.HasPrefix(e, shellEntryPoint+"=") {
 			continue
 		}
 		cleaned = append(cleaned, e)
@@ -194,126 +186,3 @@
 	}
 	return procErr
 }
-
-const ShellEntryPoint = "VEYRON_SHELL_HELPER_PROCESS_ENTRY_POINT"
-
-func RegisterChild(name, help string, main Main) {
-	child.Lock()
-	defer child.Unlock()
-	child.mains[name] = &childEntryPoint{main, help}
-}
-
-// DispatchInTest will execute the requested subproccess command from within
-// a unit test run as a subprocess.
-func DispatchInTest() {
-	if !IsTestHelperProcess() {
-		return
-	}
-	if err := child.dispatch(); err != nil {
-		vlog.Fatalf("Failed: %s", err)
-	}
-	os.Exit(0)
-}
-
-// Dispatch will execute the requested subprocess command from a within a
-// a subprocess that is not a unit test.
-func Dispatch() error {
-	if IsTestHelperProcess() {
-		return fmt.Errorf("use DispatchInTest in unittests")
-	}
-	return child.dispatch()
-}
-
-func (child *childRegistrar) hasCommand(name string) bool {
-	child.Lock()
-	_, present := child.mains[name]
-	child.Unlock()
-	return present
-}
-
-func (child *childRegistrar) dispatch() error {
-	ch, err := vexec.GetChildHandle()
-	if err != nil {
-		// This is for debugging only. It's perfectly reasonable for this
-		// error to occur if the process is started by a means other
-		// than the exec library.
-		vlog.VI(1).Infof("failed to get child handle: %s", err)
-	}
-
-	// Only signal that the child is ready or failed if we successfully get
-	// a child handle. We most likely failed to get a child handle
-	// because the subprocess was run directly from the command line.
-	command := os.Getenv(ShellEntryPoint)
-	if len(command) == 0 {
-		err := fmt.Errorf("Failed to find entrypoint %q", ShellEntryPoint)
-		if ch != nil {
-			ch.SetFailed(err)
-		}
-		return err
-	}
-
-	child.Lock()
-	m := child.mains[command]
-	child.Unlock()
-
-	if m == nil {
-		err := fmt.Errorf("Shell command %q not registered", command)
-		if ch != nil {
-			ch.SetFailed(err)
-		}
-		return err
-	}
-
-	if ch != nil {
-		ch.SetReady()
-	}
-
-	go func(pid int) {
-		for {
-			_, err := os.FindProcess(pid)
-			if err != nil {
-				vlog.Fatalf("Looks like our parent exited: %v", err)
-			}
-			time.Sleep(time.Second)
-		}
-	}(os.Getppid())
-
-	args := append([]string{command}, flag.Args()...)
-	return m.fn(os.Stdin, os.Stdout, os.Stderr, envSliceToMap(os.Environ()), args...)
-}
-
-func (child *childRegistrar) addSubprocesses(sh *Shell, pattern string) error {
-	re, err := regexp.Compile(pattern)
-	if err != nil {
-		return err
-	}
-	child.Lock()
-	defer child.Unlock()
-	found := false
-	for name, subproc := range child.mains {
-		if re.MatchString(name) {
-			sh.addSubprocess(name, subproc.help)
-			found = true
-		}
-	}
-	if !found {
-		return fmt.Errorf("patterh %q failed to match any registered commands", pattern)
-	}
-	return nil
-}
-
-// AddRegisteredSubprocesses adds any commands that match the regexp pattern
-// to the supplied shell.
-func AddRegisteredSubprocesses(sh *Shell, pattern string) error {
-	return child.addSubprocesses(sh, pattern)
-}
-
-// WaitForEof returns when a read on its io.Reader parameter returns io.EOF
-func WaitForEOF(stdin io.Reader) {
-	buf := [1024]byte{}
-	for {
-		if _, err := stdin.Read(buf[:]); err == io.EOF {
-			return
-		}
-	}
-}
diff --git a/lib/modules/modules_internal_test.go b/lib/modules/modules_internal_test.go
index 07e76bc..d0112e6 100644
--- a/lib/modules/modules_internal_test.go
+++ b/lib/modules/modules_internal_test.go
@@ -8,10 +8,6 @@
 	"testing"
 )
 
-func init() {
-	RegisterChild("echos", "[args]*", Echo)
-}
-
 func Echo(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
 	if len(args) == 0 {
 		return fmt.Errorf("no args")
@@ -30,19 +26,18 @@
 }
 
 func TestState(t *testing.T) {
-	sh := NewShell("echos")
-	sh.AddSubprocess("echonotregistered", "[args]*")
-	sh.AddFunction("echof", Echo, "[args]*")
+	sh := NewShell()
+	RegisterFunction("echoff", "[args]*", Echo)
 	assertNumHandles(t, sh, 0)
 
 	_, _ = sh.Start("echonotregistered", nil) // won't start.
 	hs, _ := sh.Start("echos", nil, "a")
-	hf, _ := sh.Start("echof", nil, "b")
+	hf, _ := sh.Start("echoff", nil, "b")
 	assertNumHandles(t, sh, 2)
 
 	for i, h := range []Handle{hs, hf} {
 		if got := h.Shutdown(nil, nil); got != nil {
-			t.Errorf("%d: got %q, want %q", i, got, nil)
+			t.Errorf("%d: got %q, want %v", i, got, nil)
 		}
 	}
 	assertNumHandles(t, sh, 0)
diff --git a/lib/modules/modules_test.go b/lib/modules/modules_test.go
index e155934..0fbab39 100644
--- a/lib/modules/modules_test.go
+++ b/lib/modules/modules_test.go
@@ -6,6 +6,7 @@
 	"fmt"
 	"io"
 	"os"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -19,8 +20,13 @@
 	testutil.Init()
 	modules.RegisterChild("envtest", "envtest: <variables to print>...", PrintFromEnv)
 	modules.RegisterChild("printenv", "printenv", PrintEnv)
-	modules.RegisterChild("echo", "[args]*", Echo)
-	modules.RegisterChild("errortest", "", ErrorMain)
+	modules.RegisterChild("echos", "[args]*", Echo)
+	modules.RegisterChild("errortestChild", "", ErrorMain)
+
+	modules.RegisterFunction("envtestf", "envtest: <variables to print>...", PrintFromEnv)
+	modules.RegisterFunction("echof", "[args]*", Echo)
+	modules.RegisterFunction("errortestFunc", "", ErrorMain)
+
 }
 
 func Echo(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
@@ -111,7 +117,7 @@
 }
 
 func TestChild(t *testing.T) {
-	sh := modules.NewShell("envtest")
+	sh := modules.NewShell()
 	defer sh.Cleanup(nil, nil)
 	key, val := "simpleVar", "foo & bar"
 	sh.SetVar(key, val)
@@ -125,24 +131,23 @@
 	sh.SetVar(key, val)
 	testCommand(t, sh, "envtest", key, val)
 	_, err := sh.Start("non-existent-command", nil, "random", "args")
-	if err == nil || err.Error() != `Shell command "non-existent-command" not registered` {
-		t.Fatalf("unexpected error %v", err)
+	if err == nil {
+		t.Fatalf("expected error")
 	}
 }
 
 func TestFunction(t *testing.T) {
-	sh := modules.NewShell(".*")
+	sh := modules.NewShell()
 	defer sh.Cleanup(nil, nil)
 	key, val := "simpleVar", "foo & bar & baz"
 	sh.SetVar(key, val)
-	sh.AddFunction("envtestf", PrintFromEnv, "envtest: <variables to print>...")
 	testCommand(t, sh, "envtestf", key, val)
 }
 
 func TestErrorChild(t *testing.T) {
-	sh := modules.NewShell("errortest")
+	sh := modules.NewShell()
 	defer sh.Cleanup(nil, nil)
-	h, err := sh.Start("errortest", nil)
+	h, err := sh.Start("errortestChild", nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
@@ -151,16 +156,16 @@
 	}
 }
 
-func testShutdown(t *testing.T, sh *modules.Shell, isfunc bool) {
+func testShutdown(t *testing.T, sh *modules.Shell, command string, isfunc bool) {
 	result := ""
 	args := []string{"a", "b c", "ddd"}
-	if _, err := sh.Start("echo", nil, args...); err != nil {
+	if _, err := sh.Start(command, nil, args...); err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
 	var stdoutBuf bytes.Buffer
 	var stderrBuf bytes.Buffer
 	sh.Cleanup(&stdoutBuf, &stderrBuf)
-	stdoutOutput, stderrOutput := "stdout: echo\n", "stderr: echo\n"
+	stdoutOutput, stderrOutput := "stdout: "+command+"\n", "stderr: "+command+"\n"
 	for _, a := range args {
 		stdoutOutput += fmt.Sprintf("stdout: %s\n", a)
 		stderrOutput += fmt.Sprintf("stderr: %s\n", a)
@@ -177,21 +182,17 @@
 }
 
 func TestShutdownSubprocess(t *testing.T) {
-	sh := modules.NewShell("echo")
-	testShutdown(t, sh, false)
+	testShutdown(t, modules.NewShell(), "echos", false)
 }
 
 func TestShutdownFunction(t *testing.T) {
-	sh := modules.NewShell()
-	sh.AddFunction("echo", Echo, "[args]*")
-	testShutdown(t, sh, true)
+	testShutdown(t, modules.NewShell(), "echof", true)
 }
 
 func TestErrorFunc(t *testing.T) {
 	sh := modules.NewShell()
 	defer sh.Cleanup(nil, nil)
-	sh.AddFunction("errortest", ErrorMain, "")
-	h, err := sh.Start("errortest", nil)
+	h, err := sh.Start("errortestFunc", nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
@@ -210,7 +211,7 @@
 }
 
 func TestEnvelope(t *testing.T) {
-	sh := modules.NewShell("printenv")
+	sh := modules.NewShell()
 	defer sh.Cleanup(nil, nil)
 	sh.SetVar("a", "1")
 	sh.SetVar("b", "2")
@@ -258,7 +259,7 @@
 }
 
 func TestEnvMerge(t *testing.T) {
-	sh := modules.NewShell("printenv")
+	sh := modules.NewShell()
 	defer sh.Cleanup(nil, nil)
 	sh.SetVar("a", "1")
 	os.Setenv("a", "wrong, should be 1")
@@ -285,14 +286,23 @@
 	}
 }
 
+func TestSetEntryPoint(t *testing.T) {
+	env := map[string]string{"a": "a", "b": "b"}
+	nenv := modules.SetEntryPoint(env, "eg1")
+	if got, want := len(nenv), 3; got != want {
+		t.Errorf("got %d, want %d", got, want)
+	}
+	nenv = modules.SetEntryPoint(env, "eg2")
+	if got, want := len(nenv), 3; got != want {
+		t.Errorf("got %d, want %d", got, want)
+	}
+	if got, want := nenv, []string{"a=a", "b=b", "VEYRON_SHELL_HELPER_PROCESS_ENTRY_POINT=eg2"}; !reflect.DeepEqual(got, want) {
+		t.Errorf("got %d, want %d", got, want)
+	}
+}
+
 func TestHelperProcess(t *testing.T) {
 	modules.DispatchInTest()
 }
 
-// TODO(cnicolaou): more complete tests for environment variables,
-// OS being overridden by Shell for example.
-//
-// TODO(cnicolaou): test for one or either of the io.Writers being nil
-// on calls to Shutdown
-//
 // TODO(cnicolaou): test for error return from cleanup
diff --git a/lib/modules/registry.go b/lib/modules/registry.go
new file mode 100644
index 0000000..61325d2
--- /dev/null
+++ b/lib/modules/registry.go
@@ -0,0 +1,184 @@
+package modules
+
+import (
+	"flag"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+	"sync"
+	"time"
+
+	"veyron.io/veyron/veyron2/vlog"
+
+	vexec "veyron.io/veyron/veyron/lib/exec"
+)
+
+type commandDesc struct {
+	factory func() command
+	main    Main
+	help    string
+}
+
+type cmdRegistry struct {
+	sync.Mutex
+	cmds map[string]*commandDesc
+}
+
+var registry = &cmdRegistry{cmds: make(map[string]*commandDesc)}
+
+func (r *cmdRegistry) addCommand(name, help string, factory func() command, main Main) {
+	r.Lock()
+	defer r.Unlock()
+	r.cmds[name] = &commandDesc{factory, main, help}
+}
+
+func (r *cmdRegistry) getCommand(name string) *commandDesc {
+	r.Lock()
+	defer r.Unlock()
+	return r.cmds[name]
+}
+
+// RegisterChild adds a new command to the registery that will be run
+// as a subprocess. It must be called before Dispatch or DispatchInTest is
+// called, typically from an init function.
+func RegisterChild(name, help string, main Main) {
+	factory := func() command { return newExecHandle(name) }
+	registry.addCommand(name, help, factory, main)
+}
+
+// RegisterFunction adds a new command to the registry that will be run
+// within the current process. It can be called at any time prior to an
+// attempt to use it.
+func RegisterFunction(name, help string, main Main) {
+	factory := func() command { return newFunctionHandle(name, main) }
+	registry.addCommand(name, help, factory, main)
+}
+
+// Help returns the help message for the specified command, or a list
+// of all commands if the command parameter is an empty string.
+func Help(command string) string {
+	return registry.help(command)
+}
+
+func (r *cmdRegistry) help(command string) string {
+	r.Lock()
+	defer r.Unlock()
+	if len(command) == 0 {
+		h := ""
+		for c, _ := range r.cmds {
+			h += c + ", "
+		}
+		return strings.TrimRight(h, ", ")
+	}
+	if c := r.cmds[command]; c != nil {
+		return command + ": " + c.help
+	}
+	return ""
+}
+
+const shellEntryPoint = "VEYRON_SHELL_HELPER_PROCESS_ENTRY_POINT"
+
+// IsModulesProcess returns true if this process is run using
+// the modules package.
+func IsModulesProcess() bool {
+	return os.Getenv(shellEntryPoint) != ""
+}
+
+// SetEntryPoint returns a slice of strings that is guaranteed to contain
+// one and only instance of the entry point environment variable set to the
+// supplied name value.
+func SetEntryPoint(env map[string]string, name string) []string {
+	newEnv := make([]string, 0, len(env)+1)
+	for k, v := range env {
+		if k == shellEntryPoint {
+			continue
+		}
+		newEnv = append(newEnv, k+"="+v)
+	}
+	return append(newEnv, shellEntryPoint+"="+name)
+}
+
+// DispatchInTest will execute the requested subproccess command from within
+// a unit test run as a subprocess.
+func DispatchInTest() {
+	if !IsTestHelperProcess() {
+		return
+	}
+	if err := registry.dispatch(); err != nil {
+		vlog.Fatalf("Failed: %s", err)
+	}
+	os.Exit(0)
+}
+
+// Dispatch will execute the requested subprocess command from a within a
+// a subprocess that is not a unit test. It will return without an error
+// if it is executed by a process that does not specify an entry point
+// in its environment.
+func Dispatch() error {
+	if !IsModulesProcess() {
+		return nil
+	}
+	if IsTestHelperProcess() {
+		return fmt.Errorf("use DispatchInTest in unittests")
+	}
+	return registry.dispatch()
+}
+
+func (r *cmdRegistry) dispatch() error {
+	ch, err := vexec.GetChildHandle()
+	if err != nil {
+		// This is for debugging only. It's perfectly reasonable for this
+		// error to occur if the process is started by a means other
+		// than the exec library.
+		vlog.VI(1).Infof("failed to get child handle: %s", err)
+	}
+
+	// Only signal that the child is ready or failed if we successfully get
+	// a child handle. We most likely failed to get a child handle
+	// because the subprocess was run directly from the command line.
+	command := os.Getenv(shellEntryPoint)
+	if len(command) == 0 {
+		err := fmt.Errorf("Failed to find entrypoint %q", command)
+		if ch != nil {
+			ch.SetFailed(err)
+		}
+		return err
+	}
+
+	m := registry.getCommand(command)
+	if m == nil {
+		err := fmt.Errorf("%s: not registered", command)
+		if ch != nil {
+			ch.SetFailed(err)
+		}
+		return err
+	}
+
+	if ch != nil {
+		ch.SetReady()
+	}
+
+	go func(pid int) {
+		for {
+			_, err := os.FindProcess(pid)
+			if err != nil {
+				vlog.Fatalf("Looks like our parent exited: %v", err)
+			}
+			time.Sleep(time.Second)
+		}
+	}(os.Getppid())
+
+	args := append([]string{command}, flag.Args()...)
+	return m.main(os.Stdin, os.Stdout, os.Stderr, envSliceToMap(os.Environ()), args...)
+}
+
+// WaitForEof returns when a read on its io.Reader parameter returns io.EOF
+func WaitForEOF(stdin io.Reader) {
+	buf := [1024]byte{}
+	for {
+		if _, err := stdin.Read(buf[:]); err == io.EOF {
+			return
+		}
+	}
+}
diff --git a/lib/modules/shell.go b/lib/modules/shell.go
index 535cc01..9e4c3b9 100644
--- a/lib/modules/shell.go
+++ b/lib/modules/shell.go
@@ -1,21 +1,15 @@
 // Package modules provides a mechanism for running commonly used services
 // as subprocesses and client functionality for accessing those services.
 // Such services and functions are collectively called 'commands' and are
-// registered with and executed within a context, defined by the Shell type.
-// The Shell is analagous to the original UNIX shell and maintains a
-// key, value store of variables that is accessible to all of the commands that
-// it hosts. These variables may be referenced by the arguments passed to
-// commands.
+// registered with a single, per-process, registry, but executed within a
+// context, defined by the Shell type. The Shell is analagous to the original
+// UNIX shell and maintains a key, value store of variables that is accessible
+// to all of the commands that it hosts. These variables may be referenced by
+// the arguments passed to commands.
 //
-// Commands are added to a shell in two ways: one for a subprocess and another
-// for an inprocess function.
-//
-// - subprocesses are added using the AddSubprocess method in the parent
-//   and by the modules.RegisterChild function in the child process (typically
-//   RegisterChild is called from an init function). modules.Dispatch must
-//   be called in the child process to execute the subprocess 'Main' function
-//   provided to RegisterChild.
-// - inprocess functions are added using the AddFunction method.
+// Commands are added to the registry in two ways:
+// - via RegisterChild for a subprocess
+// - via RegisterFunction for an in-process function
 //
 // In all cases commands are started by invoking the Start method on the
 // Shell with the name of the command to run. An instance of the Handle
@@ -42,60 +36,34 @@
 
 import (
 	"flag"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
-	"strings"
 	"sync"
 	"time"
 
 	"veyron.io/veyron/veyron/lib/exec"
 	"veyron.io/veyron/veyron/lib/flags/consts"
-	"veyron.io/veyron/veyron2/vlog"
 )
 
 // Shell represents the context within which commands are run.
 type Shell struct {
 	mu           sync.Mutex
 	env          map[string]string
-	cmds         map[string]*commandDesc
 	handles      map[Handle]struct{}
 	credDir      string
 	startTimeout time.Duration
 	config       exec.Config
 }
 
-type commandDesc struct {
-	factory func() command
-	help    string
-}
-
-type childEntryPoint struct {
-	fn   Main
-	help string
-}
-
-type childRegistrar struct {
-	sync.Mutex
-	mains map[string]*childEntryPoint
-}
-
-var child = &childRegistrar{mains: make(map[string]*childEntryPoint)}
-
 // NewShell creates a new instance of Shell. If this new instance is is a test
 // and no credentials have been configured in the environment via
 // consts.VeyronCredentials then CreateAndUseNewCredentials will be used to
-// configure a new ID for the shell and its children.  NewShell takes optional
-// regexp patterns that can be used to specify subprocess commands that are
-// implemented in the same binary as this shell (i.e. have been registered
-// using modules.RegisterChild) to be automatically added to it. If the
-// patterns fail to match any such command then they have no effect.
-func NewShell(patterns ...string) *Shell {
-	// TODO(cnicolaou): should create a new identity if one doesn't
-	// already exist
+// configure a new ID for the shell and its children.
+func NewShell() *Shell {
 	sh := &Shell{
 		env:          make(map[string]string),
-		cmds:         make(map[string]*commandDesc),
 		handles:      make(map[Handle]struct{}),
 		startTimeout: time.Minute,
 		config:       exec.NewConfig(),
@@ -106,9 +74,6 @@
 			panic(err)
 		}
 	}
-	for _, pattern := range patterns {
-		child.addSubprocesses(sh, pattern)
-	}
 	return sh
 }
 
@@ -134,51 +99,15 @@
 
 type Main func(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error
 
-// AddSubprocess adds a new command to the Shell that will be run
-// as a subprocess. In addition, the child process must call RegisterChild
-// using the same name used here and provide the function to be executed
-// in the child.
-func (sh *Shell) AddSubprocess(name, help string) {
-	if !child.hasCommand(name) {
-		vlog.Infof("Warning: %q is not registered with modules.Dispatcher", name)
-	}
-	sh.addSubprocess(name, help)
-}
-
-func (sh *Shell) addSubprocess(name string, help string) {
-	sh.mu.Lock()
-	sh.cmds[name] = &commandDesc{func() command { return newExecHandle(name) }, help}
-	sh.mu.Unlock()
-}
-
-// AddFunction adds a new command to the Shell that will be run
-// within the current process.
-func (sh *Shell) AddFunction(name string, main Main, help string) {
-	sh.mu.Lock()
-	sh.cmds[name] = &commandDesc{func() command { return newFunctionHandle(name, main) }, help}
-	sh.mu.Unlock()
-}
-
 // String returns a string representation of the Shell, which is a
 // list of the commands currently available in the shell.
 func (sh *Shell) String() string {
-	sh.mu.Lock()
-	defer sh.mu.Unlock()
-	h := ""
-	for n, _ := range sh.cmds {
-		h += n + ", "
-	}
-	return strings.TrimRight(h, ", ")
+	return registry.help("")
 }
 
 // Help returns the help message for the specified command.
 func (sh *Shell) Help(command string) string {
-	sh.mu.Lock()
-	defer sh.mu.Unlock()
-	if c := sh.cmds[command]; c != nil {
-		return command + ": " + c.help
-	}
-	return ""
+	return registry.help(command)
 }
 
 // Start starts the specified command, it returns a Handle which can be used
@@ -191,16 +120,14 @@
 // error is returned, in which case it may be used to retrieve any output
 // from the failed command.
 //
-// Commands may have already been registered with the Shell using AddFunction
-// or AddSubprocess, but if not, they will treated as subprocess commands
-// and an attempt made to run them. Such 'dynamically' started subprocess
-// commands are not remembered the by Shell and do not provide a 'help'
-// message etc; their handles are remembered and will be acted on by
-// the Cleanup method. If the non-registered subprocess command does not
-// exist then the Start command will return an error.
+// Commands must have already been registered using RegisterFunction
+// or RegisterChild.
 func (sh *Shell) Start(name string, env []string, args ...string) (Handle, error) {
 	cenv := sh.MergedEnv(env)
-	cmd := sh.getCommand(name)
+	cmd := registry.getCommand(name)
+	if cmd == nil {
+		return nil, fmt.Errorf("%s: not registered", name)
+	}
 	expanded := append([]string{name}, sh.expand(args...)...)
 	h, err := cmd.factory().start(sh, cenv, expanded...)
 	if err != nil {
@@ -214,16 +141,6 @@
 	return h, nil
 }
 
-func (sh *Shell) getCommand(name string) *commandDesc {
-	sh.mu.Lock()
-	cmd := sh.cmds[name]
-	sh.mu.Unlock()
-	if cmd == nil {
-		cmd = &commandDesc{func() command { return newExecHandle(name) }, ""}
-	}
-	return cmd
-}
-
 // SetStartTimeout sets the timeout for starting subcommands.
 func (sh *Shell) SetStartTimeout(d time.Duration) {
 	sh.startTimeout = d
@@ -233,7 +150,11 @@
 // used for running the subprocess or function if it were started with the
 // specifed arguments.
 func (sh *Shell) CommandEnvelope(name string, env []string, args ...string) ([]string, []string) {
-	return sh.getCommand(name).factory().envelope(sh, sh.MergedEnv(env), args...)
+	cmd := registry.getCommand(name)
+	if cmd == nil {
+		return []string{}, []string{}
+	}
+	return cmd.factory().envelope(sh, sh.MergedEnv(env), args...)
 }
 
 // Forget tells the Shell to stop tracking the supplied Handle. This is
diff --git a/lib/signals/signals_test.go b/lib/signals/signals_test.go
index e101277..d57c65c 100644
--- a/lib/signals/signals_test.go
+++ b/lib/signals/signals_test.go
@@ -113,7 +113,6 @@
 
 func newShell(t *testing.T, command string) (*modules.Shell, modules.Handle, *expect.Session) {
 	sh := modules.NewShell()
-	sh.AddSubprocess(command, "")
 	handle, err := sh.Start(command, nil)
 	if err != nil {
 		sh.Cleanup(os.Stderr, os.Stderr)
@@ -330,7 +329,6 @@
 	defer r.Cleanup()
 
 	sh := modules.NewShell()
-	sh.AddSubprocess("handleDefaults", "")
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 
 	// Set the child process up with a blessing from the parent so that
diff --git a/runtimes/google/ipc/client_test.go b/runtimes/google/ipc/client_test.go
index c77320f..c4e8b8a 100644
--- a/runtimes/google/ipc/client_test.go
+++ b/runtimes/google/ipc/client_test.go
@@ -26,8 +26,7 @@
 }
 
 func runMountTable(t *testing.T) (*modules.Shell, func()) {
-	sh := modules.NewShell(".*")
-	core.Install(sh)
+	sh := modules.NewShell()
 	root, err := sh.Start(core.RootMTCommand, nil, testArgs()...)
 	if err != nil {
 		t.Fatalf("unexpected error for root mt: %s", err)
diff --git a/runtimes/google/ipc/stream/manager/manager_test.go b/runtimes/google/ipc/stream/manager/manager_test.go
index 19dd942..8d4d872 100644
--- a/runtimes/google/ipc/stream/manager/manager_test.go
+++ b/runtimes/google/ipc/stream/manager/manager_test.go
@@ -544,7 +544,7 @@
 
 func TestServerRestartDuringClientLifetime(t *testing.T) {
 	client := InternalNew(naming.FixedRoutingID(0xcccccccc))
-	sh := modules.NewShell(".*")
+	sh := modules.NewShell()
 	defer sh.Cleanup(nil, nil)
 	h, err := sh.Start("runServer", nil, "127.0.0.1:0")
 	if err != nil {
@@ -583,7 +583,7 @@
 
 func TestServerRestartDuringClientLifetimeWS(t *testing.T) {
 	client := InternalNew(naming.FixedRoutingID(0xcccccccc))
-	sh := modules.NewShell(".*")
+	sh := modules.NewShell()
 	defer sh.Cleanup(nil, nil)
 	h, err := sh.Start("runServer", nil, "127.0.0.1:0")
 	if err != nil {
diff --git a/runtimes/google/rt/mgmt_test.go b/runtimes/google/rt/mgmt_test.go
index cd3db2a..defc3c7 100644
--- a/runtimes/google/rt/mgmt_test.go
+++ b/runtimes/google/rt/mgmt_test.go
@@ -116,7 +116,7 @@
 // TestNoWaiters verifies that the child process exits in the absence of any
 // wait channel being registered with its runtime.
 func TestNoWaiters(t *testing.T) {
-	sh := modules.NewShell(noWaitersCmd)
+	sh := modules.NewShell()
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 	h, err := sh.Start(noWaitersCmd, nil)
 	if err != nil {
@@ -143,7 +143,7 @@
 // TestForceStop verifies that ForceStop causes the child process to exit
 // immediately.
 func TestForceStop(t *testing.T) {
-	sh := modules.NewShell(forceStopCmd)
+	sh := modules.NewShell()
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 	h, err := sh.Start(forceStopCmd, nil)
 	if err != nil {
@@ -295,7 +295,7 @@
 
 	childcreds := security.NewVeyronCredentials(r.Principal(), appCmd)
 	configServer, configServiceName, ch := createConfigServer(t, r)
-	sh := modules.NewShell(appCmd)
+	sh := modules.NewShell()
 	sh.SetVar(consts.VeyronCredentials, childcreds)
 	sh.SetConfigKey(mgmt.ParentNameConfigKey, configServiceName)
 	sh.SetConfigKey(mgmt.ProtocolConfigKey, "tcp")
diff --git a/runtimes/google/rt/rt_test.go b/runtimes/google/rt/rt_test.go
index 84085db..ff0ae22 100644
--- a/runtimes/google/rt/rt_test.go
+++ b/runtimes/google/rt/rt_test.go
@@ -70,7 +70,7 @@
 }
 
 func TestInitArgs(t *testing.T) {
-	sh := modules.NewShell("child")
+	sh := modules.NewShell()
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 	h, err := sh.Start("child", nil, "--logtostderr=true", "--vv=3", "--", "foobar")
 	if err != nil {
@@ -136,7 +136,7 @@
 	if err != nil {
 		return err
 	}
-	sh := modules.NewShell("principal")
+	sh := modules.NewShell()
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 	h, err := sh.Start("principal", nil, args[1:]...)
 	if err != nil {
@@ -191,7 +191,7 @@
 }
 
 func TestPrincipalInheritance(t *testing.T) {
-	sh := modules.NewShell("principal", "runner")
+	sh := modules.NewShell()
 	defer func() {
 		sh.Cleanup(os.Stdout, os.Stderr)
 	}()
@@ -259,7 +259,7 @@
 		t.Fatal(err)
 	}
 
-	sh := modules.NewShell("principal")
+	sh := modules.NewShell()
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 
 	pubkey, err := collect(sh, "principal", nil)
diff --git a/runtimes/google/rt/signal_test.go b/runtimes/google/rt/signal_test.go
index de2d98c..df93748 100644
--- a/runtimes/google/rt/signal_test.go
+++ b/runtimes/google/rt/signal_test.go
@@ -73,7 +73,7 @@
 }
 
 func TestWithRuntime(t *testing.T) {
-	sh := modules.NewShell("withRuntime")
+	sh := modules.NewShell()
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 	h, err := sh.Start("withRuntime", nil)
 	if err != nil {
@@ -90,7 +90,7 @@
 }
 
 func TestWithoutRuntime(t *testing.T) {
-	sh := modules.NewShell("withoutRuntime")
+	sh := modules.NewShell()
 	defer sh.Cleanup(os.Stderr, os.Stderr)
 	h, err := sh.Start("withoutRuntime", nil)
 	if err != nil {
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index 37b5bb9..1eab0bd 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -190,16 +190,10 @@
 	// args[0] is the entrypoint for the binary to be run from the shell script
 	// that SelfInstall will write out.
 	entrypoint := args[0]
-	osenv := make([]string, 0, len(env))
-	for k, v := range env {
-		if k == modules.ShellEntryPoint {
-			// Overwrite the entrypoint in our environment (i.e. the one
-			// that got us here), with the one we want written out in the shell
-			// script.
-			v = entrypoint
-		}
-		osenv = append(osenv, k+"="+v)
-	}
+	// Overwrite the entrypoint in our environment (i.e. the one
+	// that got us here), with the one we want written out in the shell
+	// script.
+	osenv := modules.SetEntryPoint(env, entrypoint)
 	if args[1] != "--" {
 		vlog.Fatalf("expected '--' immediately following command name")
 	}
diff --git a/services/mgmt/node/impl/util_test.go b/services/mgmt/node/impl/util_test.go
index 62dc931..0db0633 100644
--- a/services/mgmt/node/impl/util_test.go
+++ b/services/mgmt/node/impl/util_test.go
@@ -60,7 +60,7 @@
 }
 
 func createShellAndMountTable(t *testing.T) (*modules.Shell, func()) {
-	sh := core.NewShell()
+	sh := modules.NewShell()
 	// The shell, will, by default share credentials with its children.
 	sh.ClearVar(consts.VeyronCredentials)
 
diff --git a/tools/naming/simulator/driver.go b/tools/naming/simulator/driver.go
index e27e4bb..9242830 100644
--- a/tools/naming/simulator/driver.go
+++ b/tools/naming/simulator/driver.go
@@ -21,7 +21,6 @@
 	"veyron.io/veyron/veyron/lib/expect"
 	"veyron.io/veyron/veyron/lib/flags/consts"
 	"veyron.io/veyron/veyron/lib/modules"
-	"veyron.io/veyron/veyron/lib/modules/core"
 	_ "veyron.io/veyron/veyron/profiles"
 )
 
@@ -101,7 +100,7 @@
 	// Subprocesses commands are run by fork/execing this binary
 	// so we must test to see if this instance is a subprocess or the
 	// the original command line instance.
-	if os.Getenv(modules.ShellEntryPoint) != "" {
+	if modules.IsModulesProcess() {
 		// Subprocess, run the requested command.
 		if err := modules.Dispatch(); err != nil {
 			fmt.Fprintf(os.Stderr, "failed: %v\n", err)
@@ -116,8 +115,6 @@
 		shell.CreateAndUseNewCredentials()
 	}
 
-	core.Install(shell)
-
 	scanner := bufio.NewScanner(os.Stdin)
 	lineno := 1
 	prompt(lineno)
diff --git a/tools/servicerunner/main.go b/tools/servicerunner/main.go
index 8620d14..00b00ff 100644
--- a/tools/servicerunner/main.go
+++ b/tools/servicerunner/main.go
@@ -14,7 +14,6 @@
 	"veyron.io/veyron/veyron/lib/expect"
 	"veyron.io/veyron/veyron/lib/flags/consts"
 	"veyron.io/veyron/veyron/lib/modules"
-	"veyron.io/veyron/veyron/lib/modules/core"
 	_ "veyron.io/veyron/veyron/profiles"
 )
 
@@ -57,9 +56,7 @@
 func main() {
 	rt.Init()
 
-	// NOTE(sadovsky): It would be better if Dispatch() itself performed the env
-	// check.
-	if os.Getenv(modules.ShellEntryPoint) != "" {
+	if modules.IsModulesProcess() {
 		panicOnError(modules.Dispatch())
 		return
 	}
@@ -78,9 +75,6 @@
 		}
 		vars[consts.VeyronCredentials] = v
 	}
-	// NOTE(sadovsky): The following line will not be needed if the modules
-	// library is restructured per my proposal.
-	core.Install(sh)
 
 	h, err := sh.Start("root", nil, "--", "--veyron.tcp.address=127.0.0.1:0")
 	panicOnError(err)