diff --git a/ctl_test.go b/ctl_test.go index 48be61d..bf5f1c4 100644 --- a/ctl_test.go +++ b/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 := queue.Init() - tcheck(t, err, "queue init") - defer queue.Shutdown() - - err = store.Init(ctxbg) + 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 diff --git a/imapserver/authenticate_test.go b/imapserver/authenticate_test.go index c05b5d7..e69b86b 100644 --- a/imapserver/authenticate_test.go +++ b/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") } diff --git a/imapserver/fuzz_test.go b/imapserver/fuzz_test.go index 66353e4..20f25b1 100644 --- a/imapserver/fuzz_test.go +++ b/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() diff --git a/imapserver/server_test.go b/imapserver/server_test.go index b6f5e9e..794e681 100644 --- a/imapserver/server_test.go +++ b/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 diff --git a/queue/queue_test.go b/queue/queue_test.go index 4443c33..a8dc3f4 100644 --- a/queue/queue_test.go +++ b/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() diff --git a/smtpserver/alias_test.go b/smtpserver/alias_test.go index 5efcbcc..5c53398 100644 --- a/smtpserver/alias_test.go +++ b/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" diff --git a/smtpserver/fuzz_test.go b/smtpserver/fuzz_test.go index f03f710..b532685 100644 --- a/smtpserver/fuzz_test.go +++ b/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) diff --git a/smtpserver/server_test.go b/smtpserver/server_test.go index 2878137..89393d0 100644 --- a/smtpserver/server_test.go +++ b/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") diff --git a/store/account.go b/store/account.go index dcec9e3..3a18483 100644 --- a/store/account.go +++ b/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 { diff --git a/store/account_test.go b/store/account_test.go index 5a32261..4c9e00f 100644 --- a/store/account_test.go +++ b/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) diff --git a/store/export_test.go b/store/export_test.go index 06afbca..951092b 100644 --- a/store/export_test.go +++ b/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") diff --git a/store/threads_test.go b/store/threads_test.go index ac1c8d9..1715c7c 100644 --- a/store/threads_test.go +++ b/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. diff --git a/webaccount/account_test.go b/webaccount/account_test.go index 9283b58..5b13e2c 100644 --- a/webaccount/account_test.go +++ b/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) diff --git a/webmail/api_test.go b/webmail/api_test.go index 171af43..63732ec 100644 --- a/webmail/api_test.go +++ b/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) diff --git a/webmail/view_test.go b/webmail/view_test.go index bad209b..7f68deb 100644 --- a/webmail/view_test.go +++ b/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) diff --git a/webmail/webmail_test.go b/webmail/webmail_test.go index d90ca3e..56485b9 100644 --- a/webmail/webmail_test.go +++ b/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/"}