In tests, make initializing store/, its switchboard and an account more consistent.

Initialize store and switchboard first, then open account, and close in reverse
order.

Replace all "CheckClosed" calls with "WaitClosed", future changings will keep
an account reference open for a bit after the last regular close, so we can't
know that an account should be closed during tests.

Remove one parameter from the (still too long) "start test server" function in
imapserver testing code.

ctl_test.go

@@ -49,15 +49,14 @@ func TestCtl(t *testing.T) {
 	if errs := mox.LoadConfig(ctxbg, pkglog, true, false); len(errs) > 0 {
 		t.Fatalf("loading mox config: %v", errs)
 	}
-	defer store.Switchboard()()
+	err := store.Init(ctxbg)
-
-	err := queue.Init()
-	tcheck(t, err, "queue init")
-	defer queue.Shutdown()
-
-	err = store.Init(ctxbg)
 	tcheck(t, err, "store init")
 	defer store.Close()
+	defer store.Switchboard()()
+
+	err = queue.Init()
+	tcheck(t, err, "queue init")
+	defer queue.Shutdown()
 
 	var cid int64
 

imapserver/authenticate_test.go

@@ -240,7 +240,7 @@ func TestAuthenticateCRAMMD5(t *testing.T) {
 }
 
 func TestAuthenticateTLSClientCert(t *testing.T) {
-	tc := startArgs(t, true, true, true, true, "mjl")
+	tc := startArgsMore(t, true, true, nil, nil, true, true, "mjl", nil)
 	tc.transactf("no", "authenticate external ") // No TLS auth.
 	tc.close()
 
@@ -263,7 +263,7 @@ func TestAuthenticateTLSClientCert(t *testing.T) {
 	}
 
 	// No preauth, explicit authenticate with TLS.
-	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, true, "mjl", addClientCert)
+	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, "mjl", addClientCert)
 	if tc.client.Preauth {
 		t.Fatalf("preauthentication while not configured for tls public key")
 	}
@@ -271,7 +271,7 @@ func TestAuthenticateTLSClientCert(t *testing.T) {
 	tc.close()
 
 	// External with explicit username.
-	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, true, "mjl", addClientCert)
+	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, "mjl", addClientCert)
 	if tc.client.Preauth {
 		t.Fatalf("preauthentication while not configured for tls public key")
 	}
@@ -279,12 +279,12 @@ func TestAuthenticateTLSClientCert(t *testing.T) {
 	tc.close()
 
 	// No preauth, also allow other mechanisms.
-	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, true, "mjl", addClientCert)
+	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, "mjl", addClientCert)
 	tc.transactf("ok", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000mjl@mox.example\u0000"+password0)))
 	tc.close()
 
 	// No preauth, also allow other username for same account.
-	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, true, "mjl", addClientCert)
+	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, "mjl", addClientCert)
 	tc.transactf("ok", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000móx@mox.example\u0000"+password0)))
 	tc.close()
 
@@ -295,12 +295,12 @@ func TestAuthenticateTLSClientCert(t *testing.T) {
 	tcheck(t, err, "set password")
 	err = acc.Close()
 	tcheck(t, err, "close account")
-	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, true, "mjl", addClientCert)
+	tc = startArgsMore(t, true, true, nil, &clientConfig, false, true, "mjl", addClientCert)
 	tc.transactf("no", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000other@mox.example\u0000test1234")))
 	tc.close()
 
 	// Starttls and external auth.
-	tc = startArgsMore(t, true, false, nil, &clientConfig, false, true, true, "mjl", addClientCert)
+	tc = startArgsMore(t, true, false, nil, &clientConfig, false, true, "mjl", addClientCert)
 	tc.client.Starttls(&clientConfig)
 	tc.transactf("ok", "authenticate external =")
 	tc.close()
@@ -318,7 +318,7 @@ func TestAuthenticateTLSClientCert(t *testing.T) {
 	defer cancel()
 	mox.StartTLSSessionTicketKeyRefresher(ctx, pkglog, &serverConfig)
 	clientConfig.ClientSessionCache = tls.NewLRUClientSessionCache(10)
-	tc = startArgsMore(t, true, true, &serverConfig, &clientConfig, false, true, true, "mjl", addClientCert)
+	tc = startArgsMore(t, true, true, &serverConfig, &clientConfig, false, true, "mjl", addClientCert)
 	if !tc.client.Preauth {
 		t.Fatalf("not preauthentication while configured for tls public key")
 	}
@@ -330,7 +330,7 @@ func TestAuthenticateTLSClientCert(t *testing.T) {
 	tc.close()
 
 	// Authentication works with TLS resumption.
-	tc = startArgsMore(t, true, true, &serverConfig, &clientConfig, false, true, true, "mjl", addClientCert)
+	tc = startArgsMore(t, true, true, &serverConfig, &clientConfig, false, true, "mjl", addClientCert)
 	if !tc.client.Preauth {
 		t.Fatalf("not preauthentication while configured for tls public key")
 	}

imapserver/fuzz_test.go

@@ -69,6 +69,8 @@ func FuzzServer(f *testing.F) {
 	if err != nil {
 		f.Fatalf("store init: %v", err)
 	}
+	defer store.Switchboard()()
+
 	acc, err := store.OpenAccount(log, "mjl", false)
 	if err != nil {
 		f.Fatalf("open account: %v", err)
@@ -81,7 +83,6 @@ func FuzzServer(f *testing.F) {
 	if err != nil {
 		f.Fatalf("set password: %v", err)
 	}
-	defer store.Switchboard()()
 
 	comm := store.RegisterComm(acc)
 	defer comm.Unregister()

imapserver/server_test.go

@@ -338,7 +338,6 @@ func (tc *testconn) close0(waitclose bool) {
 	if waitclose {
 		tc.account.WaitClosed()
 	}
-	// no account.CheckClosed(), the tests open accounts multiple times.
 	tc.account = nil
 	tc.serverConn.Close()
 	tc.waitDone()
@@ -385,7 +384,7 @@ const password0 = "te\u0301st \u00a0\u2002\u200a" // NFD and various unicode spa
 const password1 = "tést    "                      // PRECIS normalized, with NFC.
 
 func startArgs(t *testing.T, first, immediateTLS bool, allowLoginWithoutTLS, setPassword bool, accname string) *testconn {
-	return startArgsMore(t, first, immediateTLS, nil, nil, allowLoginWithoutTLS, false, setPassword, accname, nil)
+	return startArgsMore(t, first, immediateTLS, nil, nil, allowLoginWithoutTLS, setPassword, accname, nil)
 }
 
 // namedConn wraps a conn so it can return a RemoteAddr with a non-empty name.
@@ -400,9 +399,10 @@ func (c namedConn) RemoteAddr() net.Addr {
 }
 
 // todo: the parameters and usage are too much now. change to scheme similar to smtpserver, with params in a struct, and a separate method for init and making a connection.
-func startArgsMore(t *testing.T, first, immediateTLS bool, serverConfig, clientConfig *tls.Config, allowLoginWithoutTLS, noCloseSwitchboard, setPassword bool, accname string, afterInit func() error) *testconn {
+func startArgsMore(t *testing.T, first, immediateTLS bool, serverConfig, clientConfig *tls.Config, allowLoginWithoutTLS, setPassword bool, accname string, afterInit func() error) *testconn {
 	limitersInit() // Reset rate limiters.
 
+	switchStop := func() {}
 	if first {
 		mox.ConfigStaticPath = filepath.FromSlash("../testdata/imap/mox.conf")
 		mox.MustLoadConfig(true, false)
@@ -410,17 +410,16 @@ func startArgsMore(t *testing.T, first, immediateTLS bool, serverConfig, clientC
 		os.RemoveAll("../testdata/imap/data")
 		err := store.Init(ctxbg)
 		tcheck(t, err, "store init")
+		switchStop = store.Switchboard()
 	}
+
 	acc, err := store.OpenAccount(pkglog, accname, false)
 	tcheck(t, err, "open account")
 	if setPassword {
 		err = acc.SetPassword(pkglog, password0)
 		tcheck(t, err, "set password")
 	}
-	switchStop := func() {}
 	if first {
-		switchStop = store.Switchboard()
-
 		// Add a deleted mailbox, may excercise some code paths.
 		err = acc.DB.Write(ctxbg, func(tx *bstore.Tx) error {
 			// todo: add a message to inbox and remove it again. need to change all uids in the tests.
@@ -487,15 +486,12 @@ func startArgsMore(t *testing.T, first, immediateTLS bool, serverConfig, clientC
 	go func() {
 		const viaHTTPS = false
 		serve("test", cid, serverConfig, serverConn, immediateTLS, allowLoginWithoutTLS, viaHTTPS, "")
-		if !noCloseSwitchboard {
-			switchStop()
-		}
 		close(done)
 	}()
 	client, err := imapclient.New(connCounter, clientConn, true)
 	tcheck(t, err, "new client")
 	tc := &testconn{t: t, conn: clientConn, client: client, done: done, serverConn: serverConn, account: acc}
-	if first && noCloseSwitchboard {
+	if first {
 		tc.switchStop = switchStop
 	}
 	return tc

queue/queue_test.go

@@ -61,18 +61,20 @@ func setup(t *testing.T) (*store.Account, func()) {
 	mox.Context = ctxbg
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/queue/mox.conf")
 	mox.MustLoadConfig(true, false)
+	mox.Shutdown, mox.ShutdownCancel = context.WithCancel(ctxbg)
 	err := Init()
 	tcheck(t, err, "queue init")
 	err = mtastsdb.Init(false)
 	tcheck(t, err, "mtastsdb init")
 	err = tlsrptdb.Init()
 	tcheck(t, err, "tlsrptdb init")
+	switchStop := store.Switchboard()
+
 	acc, err := store.OpenAccount(log, "mjl", false)
 	tcheck(t, err, "open account")
 	err = acc.SetPassword(log, "testtest")
 	tcheck(t, err, "set password")
-	switchStop := store.Switchboard()
+
-	mox.Shutdown, mox.ShutdownCancel = context.WithCancel(ctxbg)
 	return acc, func() {
 		acc.Close()
 		acc.WaitClosed()

smtpserver/alias_test.go

@@ -81,7 +81,7 @@ func TestAliasSubmitMsgFromDenied(t *testing.T) {
 	tcheck(t, err, "set password")
 	err = acc.Close()
 	tcheck(t, err, "close account")
-	acc.CheckClosed()
+	acc.WaitClosed()
 
 	ts.submission = true
 	ts.user = "☺@mox.example"

smtpserver/fuzz_test.go

@@ -42,6 +42,7 @@ func FuzzServer(f *testing.F) {
 	if err != nil {
 		f.Fatalf("store init: %v", err)
 	}
+	defer store.Switchboard()()
 
 	acc, err := store.OpenAccount(log, "mjl", false)
 	if err != nil {
@@ -55,7 +56,7 @@ func FuzzServer(f *testing.F) {
 	if err != nil {
 		f.Fatalf("set password: %v", err)
 	}
-	defer store.Switchboard()()
+
 	err = queue.Init()
 	if err != nil {
 		f.Fatalf("queue init: %v", err)

smtpserver/server_test.go

@@ -155,15 +155,15 @@ func newTestServer(t *testing.T, configPath string, resolver dns.Resolver) *test
 	err = store.Init(ctxbg)
 	tcheck(t, err, "store init")
 
+	ts.switchStop = store.Switchboard()
+	err = queue.Init()
+	tcheck(t, err, "queue init")
+
 	ts.acc, err = store.OpenAccount(log, "mjl", false)
 	tcheck(t, err, "open account")
 	err = ts.acc.SetPassword(log, password0)
 	tcheck(t, err, "set password")
 
-	ts.switchStop = store.Switchboard()
-	err = queue.Init()
-	tcheck(t, err, "queue init")
-
 	ts.comm = store.RegisterComm(ts.acc)
 
 	return &ts
@@ -177,15 +177,15 @@ func (ts *testserver) close() {
 	tcheck(ts.t, err, "dmarcdb close")
 	err = tlsrptdb.Close()
 	tcheck(ts.t, err, "tlsrptdb close")
-	err = store.Close()
-	tcheck(ts.t, err, "store close")
 	ts.comm.Unregister()
 	queue.Shutdown()
-	ts.switchStop()
 	err = ts.acc.Close()
 	tcheck(ts.t, err, "closing account")
 	ts.acc.WaitClosed()
 	ts.acc = nil
+	ts.switchStop()
+	err = store.Close()
+	tcheck(ts.t, err, "store close")
 }
 
 func (ts *testserver) checkCount(mailboxName string, expect int) {
@@ -1505,7 +1505,7 @@ func TestCatchall(t *testing.T) {
 	tcheck(t, err, "open account")
 	defer func() {
 		acc.Close()
-		acc.CheckClosed()
+		acc.WaitClosed()
 	}()
 	n, err = bstore.QueryDB[store.Message](ctxbg, acc.DB).Count()
 	tcheck(t, err, "checking delivered messages to catchall account")

store/account.go

@@ -965,8 +965,8 @@ var InitialUIDValidity = func() uint32 {
 }
 
 var openAccounts = struct {
-	names map[string]*Account
 	sync.Mutex
+	names map[string]*Account
 }{
 	names: map[string]*Account{},
 }
@@ -1029,6 +1029,7 @@ func OpenAccount(log mlog.Log, name string, checkLoginDisabled bool) (*Account,
 }
 
 // openAccount opens an existing account, or creates it if it is missing.
+// Called with openAccounts lock held.
 func openAccount(log mlog.Log, name string) (a *Account, rerr error) {
 	dir := filepath.Join(mox.DataDirPath("accounts"), name)
 	return OpenAccountDB(log, dir, name)
@@ -1582,15 +1583,6 @@ func (a *Account) WaitClosed() {
 	<-a.closed
 }
 
-// CheckClosed asserts that the account has a zero reference count. For use in tests.
-func (a *Account) CheckClosed() {
-	openAccounts.Lock()
-	defer openAccounts.Unlock()
-	if a.nused != 0 {
-		panic(fmt.Sprintf("account still in use, %d refs", a.nused))
-	}
-}
-
 // Close reduces the reference count, and closes the database connection when
 // it was the last user.
 func (a *Account) Close() error {

store/account_test.go

@@ -41,6 +41,13 @@ func TestMailbox(t *testing.T) {
 	os.RemoveAll("../testdata/store/data")
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/store/mox.conf")
 	mox.MustLoadConfig(true, false)
+	err := Init(ctxbg)
+	tcheck(t, err, "init")
+	defer func() {
+		err := Close()
+		tcheck(t, err, "close")
+	}()
+	defer Switchboard()()
 	acc, err := OpenAccount(log, "mjl", false)
 	tcheck(t, err, "open account")
 	defer func() {
@@ -48,7 +55,6 @@ func TestMailbox(t *testing.T) {
 		tcheck(t, err, "closing account")
 		acc.WaitClosed()
 	}()
-	defer Switchboard()()
 
 	msgFile, err := CreateMessageTemp(log, "account-test")
 	tcheck(t, err, "create temp message file")
@@ -316,12 +322,20 @@ func TestNextMessageID(t *testing.T) {
 	os.RemoveAll("../testdata/store/data")
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/store/mox.conf")
 	mox.MustLoadConfig(true, false)
+	err := Init(ctxbg)
+	tcheck(t, err, "init")
+	defer func() {
+		err := Close()
+		tcheck(t, err, "close")
+	}()
+	defer Switchboard()()
 
 	// Ensure account exists.
 	acc, err := OpenAccount(log, "mjl", false)
 	tcheck(t, err, "open account")
 	err = acc.Close()
 	tcheck(t, err, "closing account")
+	acc.WaitClosed()
 	acc = nil
 
 	// Create file on disk to occupy the first Message.ID that would otherwise be used for deliveries..
@@ -342,7 +356,6 @@ func TestNextMessageID(t *testing.T) {
 	// Open account. This should increase the next message ID.
 	acc, err = OpenAccount(log, "mjl", false)
 	tcheck(t, err, "open account")
-	defer Switchboard()()
 
 	// Deliver a message. It should get ID 2.
 	mf, err := CreateMessageTemp(log, "account-test")
@@ -369,6 +382,7 @@ func TestNextMessageID(t *testing.T) {
 
 	err = acc.Close()
 	tcheck(t, err, "closing account")
+	acc.WaitClosed()
 
 	// Try again, but also create next message directory, but no file.
 	os.MkdirAll(filepath.Join(msgDir, "b"), 0700)

store/export_test.go

@@ -24,14 +24,14 @@ func TestExport(t *testing.T) {
 	os.RemoveAll("../testdata/store/data")
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/store/mox.conf")
 	mox.MustLoadConfig(true, false)
+	defer Switchboard()()
 	acc, err := OpenAccount(pkglog, "mjl", false)
 	tcheck(t, err, "open account")
 	defer func() {
 		err := acc.Close()
 		log.Check(err, "closing account")
-		acc.CheckClosed()
+		acc.WaitClosed()
 	}()
-	defer Switchboard()()
 
 	msgFile, err := CreateMessageTemp(pkglog, "mox-test-export")
 	tcheck(t, err, "create temp")

store/threads_test.go

@@ -19,14 +19,15 @@ func TestThreadingUpgrade(t *testing.T) {
 	os.RemoveAll("../testdata/store/data")
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/store/mox.conf")
 	mox.MustLoadConfig(true, false)
+	defer Switchboard()()
+
 	acc, err := OpenAccount(log, "mjl", true)
 	tcheck(t, err, "open account")
 	defer func() {
 		err = acc.Close()
 		tcheck(t, err, "closing account")
-		acc.CheckClosed()
+		acc.WaitClosed()
 	}()
-	defer Switchboard()()
 
 	// New account already has threading. Add some messages, check the threading.
 	deliver := func(recv time.Time, s string, expThreadID int64) Message {
@@ -120,7 +121,7 @@ func TestThreadingUpgrade(t *testing.T) {
 	dbpath := acc.DBPath
 	err = acc.Close()
 	tcheck(t, err, "close account")
-	acc.CheckClosed()
+	acc.WaitClosed()
 
 	// Now clear the threading upgrade, and the threading fields and close the account.
 	// We open the database file directly, so we don't trigger the consistency checker.

webaccount/account_test.go

@@ -93,6 +93,7 @@ func tcompare(t *testing.T, got, expect any) {
 }
 
 func TestAccount(t *testing.T) {
+	log := mlog.New("webaccount", nil)
 	os.RemoveAll("../testdata/httpaccount/data")
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/httpaccount/mox.conf")
 	mox.ConfigDynamicPath = filepath.Join(filepath.Dir(mox.ConfigStaticPath), "domains.conf")
@@ -103,7 +104,8 @@ func TestAccount(t *testing.T) {
 		err := store.Close()
 		tcheck(t, err, "store close")
 	}()
-	log := mlog.New("webaccount", nil)
+	defer store.Switchboard()()
+
 	acc, err := store.OpenAccount(log, "mjl☺", false)
 	tcheck(t, err, "open account")
 	err = acc.SetPassword(log, "test1234")
@@ -113,7 +115,6 @@ func TestAccount(t *testing.T) {
 		tcheck(t, err, "closing account")
 		acc.WaitClosed()
 	}()
-	defer store.Switchboard()()
 
 	api := Account{cookiePath: "/account/"}
 	apiHandler, err := makeSherpaHandler(api.cookiePath, false)

webmail/api_test.go

@@ -56,13 +56,13 @@ func TestAPI(t *testing.T) {
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/webmail/mox.conf")
 	mox.ConfigDynamicPath = filepath.FromSlash("../testdata/webmail/domains.conf")
 	mox.MustLoadConfig(true, false)
-	defer store.Switchboard()()
 	err := store.Init(ctxbg)
 	tcheck(t, err, "store init")
 	defer func() {
 		err := store.Close()
 		tcheck(t, err, "store close")
 	}()
+	defer store.Switchboard()()
 
 	log := mlog.New("webmail", nil)
 	err = mtastsdb.Init(false)

webmail/view_test.go

@@ -29,13 +29,13 @@ func TestView(t *testing.T) {
 	mox.Context = ctxbg
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/webmail/mox.conf")
 	mox.MustLoadConfig(true, false)
-	defer store.Switchboard()()
 	err := store.Init(ctxbg)
 	tcheck(t, err, "store init")
 	defer func() {
 		err := store.Close()
 		tcheck(t, err, "store close")
 	}()
+	defer store.Switchboard()()
 
 	log := mlog.New("webmail", nil)
 	acc, err := store.OpenAccount(log, "mjl", false)

webmail/webmail_test.go

@@ -308,13 +308,13 @@ func TestWebmail(t *testing.T) {
 	mox.Context = ctxbg
 	mox.ConfigStaticPath = filepath.FromSlash("../testdata/webmail/mox.conf")
 	mox.MustLoadConfig(true, false)
-	defer store.Switchboard()()
 	err := store.Init(ctxbg)
 	tcheck(t, err, "store init")
 	defer func() {
 		err := store.Close()
 		tcheck(t, err, "store close")
 	}()
+	defer store.Switchboard()()
 
 	log := mlog.New("webmail", nil)
 	acc, err := store.OpenAccount(pkglog, "mjl", false)
@@ -324,7 +324,7 @@ func TestWebmail(t *testing.T) {
 	defer func() {
 		err := acc.Close()
 		pkglog.Check(err, "closing account")
-		acc.CheckClosed()
+		acc.WaitClosed()
 	}()
 
 	api := Webmail{maxMessageSize: 1024 * 1024, cookiePath: "/webmail/"}