go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
44.6k stars 5.45k forks source link

A deadlock code path #5116

Closed typeless closed 5 years ago

typeless commented 5 years ago

Looking at models/repo.go:2439 in the backtrace, you can see that this is in an xorm session. But at models/repo.go:2004 GetRepositoryByID, it uses the global db connection in the middle of the session. IIUC, since the session has not completed yet, this GetRepositoryByID would always fail (or block if SQLite's unlock_noitfy is implemented). ...

Screenshots

typeless commented 5 years ago

@lunny Applied https://github.com/go-gitea/gitea/pull/5118 and it works for my tests.

Here is probably another.

goroutine 19561 [chan receive, locked to thread]:
code.gitea.io/gitea/vendor/github.com/mattn/go-sqlite3.unlock_notify_wait(0x7fe3440044a8, 0x7fe344014728, 0x0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/mattn/go-sqlite3/sqlite3_opt_unlock_notify.go:96 +0x1f5
code.gitea.io/gitea/vendor/github.com/mattn/go-sqlite3._cgoexpwrap_f67dad5ac488_unlock_notify_wait(0x7fe3440044a8, 0x7fe344014728, 0x7fe344014728)
    _cgo_gotypes.go:1369 +0x35
code.gitea.io/gitea/vendor/github.com/mattn/go-sqlite3._Cfunc_sqlite3_step_internal(0x7fe344014728, 0x7fe300000000)
    _cgo_gotypes.go:1102 +0x49
code.gitea.io/gitea/vendor/github.com/mattn/go-sqlite3.(*SQLiteRows).Next.func1(0x7fe344014728, 0x1336310)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/mattn/go-sqlite3/sqlite3.go:2005 +0x56
code.gitea.io/gitea/vendor/github.com/mattn/go-sqlite3.(*SQLiteRows).Next(0xc001bddd60, 0xc00202c000, 0x27, 0x27, 0x0, 0x0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/mattn/go-sqlite3/sqlite3.go:2005 +0xa7
database/sql.(*Rows).nextLocked(0xc0018dde00, 0xc001cf0000)
    /home/typeless/tools/go/src/database/sql/sql.go:2665 +0xbc
database/sql.(*Rows).Next.func1()
    /home/typeless/tools/go/src/database/sql/sql.go:2643 +0x3c
database/sql.withLock(0x14564a0, 0xc0018dde30, 0xc0029e2040)
    /home/typeless/tools/go/src/database/sql/sql.go:3075 +0x63
database/sql.(*Rows).Next(0xc0018dde00, 0x13350a8)
    /home/typeless/tools/go/src/database/sql/sql.go:2642 +0x87
code.gitea.io/gitea/vendor/github.com/go-xorm/xorm.(*Session).nocacheGet(0xc00279b900, 0x19, 0xc0030fa000, 0x12c3b20, 0xc00211c800, 0xc00294aa00, 0x271, 0xc0009f2750, 0x1, 0x1, ...)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-xorm/xorm/session_get.go:79 +0x131
code.gitea.io/gitea/vendor/github.com/go-xorm/xorm.(*Session).get(0xc00279b900, 0x12c3b20, 0xc00211c800, 0x11231c0, 0xc0023257d0, 0xc00279b900)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-xorm/xorm/session_get.go:69 +0x397
code.gitea.io/gitea/vendor/github.com/go-xorm/xorm.(*Session).Get(0xc00279b900, 0x12c3b20, 0xc00211c800, 0xc00279b900, 0x0, 0x0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-xorm/xorm/session_get.go:22 +0x59
code.gitea.io/gitea/models.getUserByID(0x14755a0, 0xc0004c0000, 0x2, 0xc00186fec0, 0x1, 0x1)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/user.go:1164 +0xae
code.gitea.io/gitea/models.(*Repository).getOwner(0xc0028ac480, 0x14755a0, 0xc0004c0000, 0x1a, 0xc0022841a0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:463 +0x4e
code.gitea.io/gitea/models.(*Repository).mustOwner(0xc0028ac480, 0x14755a0, 0xc0004c0000, 0x199)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:473 +0x43
code.gitea.io/gitea/models.(*Repository).MustOwner(0xc0028ac480, 0x19)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:226 +0x43
code.gitea.io/gitea/models.(*Repository).cloneLink(0xc0028ac480, 0x0, 0x0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:907 +0x95
code.gitea.io/gitea/models.(*Repository).CloneLink(0xc0028ac480, 0x12c8ca0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:922 +0x30
code.gitea.io/gitea/models.(*Repository).innerAPIFormat(0xc0028ac480, 0x1475640, 0xc00279a280, 0x4, 0x1, 0xc0029e2650)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:271 +0x40
code.gitea.io/gitea/models.(*Repository).innerAPIFormat(0xc0028ac180, 0x1475640, 0xc00279a280, 0x4, 0x0, 0x0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:283 +0x5f4
code.gitea.io/gitea/models.createRepository(0xc00279a280, 0xc000c1ba00, 0xc000bef400, 0xc0028ac180, 0x0, 0xc00333e240)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:1393 +0xab4
code.gitea.io/gitea/models.ForkRepository(0xc000c1ba00, 0xc000bef400, 0xc002f83080, 0xc002fc9a2d, 0x5, 0x0, 0x0, 0x0, 0x0, 0x0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/models/repo.go:2443 +0x270
code.gitea.io/gitea/routers/repo.ForkPost(0xc000c2bd00, 0x3, 0xc002fc9a2d, 0x5, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /home/typeless/devel/go/src/code.gitea.io/gitea/routers/repo/pull.go:190 +0x3e8
reflect.Value.call(0x1140a40, 0x1333538, 0x13, 0x12d296a, 0x4, 0xc001e0f830, 0x2, 0x2, 0x12c4020, 0x874b01, ...)
    /home/typeless/tools/go/src/reflect/value.go:447 +0x446
reflect.Value.Call(0x1140a40, 0x1333538, 0x13, 0xc001e0f830, 0x2, 0x2, 0x0, 0x0, 0x0)
    /home/typeless/tools/go/src/reflect/value.go:308 +0xa4
code.gitea.io/gitea/vendor/github.com/go-macaron/inject.(*injector).callInvoke(0xc002967440, 0x1140a40, 0x1333538, 0x147c120, 0x1140a40, 0x2, 0x0, 0x0, 0x0, 0x0, ...)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:177 +0x29c
code.gitea.io/gitea/vendor/github.com/go-macaron/inject.(*injector).Invoke(0xc002967440, 0x1140a40, 0x1333538, 0x0, 0x0, 0x0, 0x0, 0x0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:137 +0xca
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Context).run(0xc0001b3e60)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:121 +0x79
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Context).Next(0xc0001b3e60)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:112 +0x2f
code.gitea.io/gitea/vendor/github.com/go-macaron/session.Sessioner.func1(0xc0001b3e60)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/session/session.go:186 +0x2df
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.ContextInvoker.Invoke(0xc0006c4aa0, 0xc0015ff870, 0x1, 0x1, 0xc0001b3e60, 0x16, 0x7fe3a5ef55b8, 0x2, 0x2)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:79 +0x51
code.gitea.io/gitea/vendor/github.com/go-macaron/inject.(*injector).fastInvoke(0xc002967440, 0x7fe3a5ef55b8, 0xc0006c4aa0, 0x147c120, 0x118b660, 0x1, 0x0, 0x0, 0x0, 0x0, ...)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:157 +0x234
code.gitea.io/gitea/vendor/github.com/go-macaron/inject.(*injector).Invoke(0xc002967440, 0x118b660, 0xc0006c4aa0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:135 +0x1b9
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Context).run(0xc0001b3e60)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:121 +0x79
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Context).Next(0xc0001b3e60)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:112 +0x2f
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.Recovery.func1(0xc0001b3e60, 0xc00011c780)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/recovery.go:161 +0x5b
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.LoggerInvoker.Invoke(0x1334e40, 0xc002967480, 0x2, 0x2, 0xc00011c780, 0x16, 0x7fe3a5ef5598, 0xc0029e3388, 0x3)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/logger.go:40 +0x74
code.gitea.io/gitea/vendor/github.com/go-macaron/inject.(*injector).fastInvoke(0xc002967440, 0x7fe3a5ef5598, 0x1334e40, 0x147c120, 0x119d960, 0x2, 0xc00011c780, 0xc0004f8d00, 0x3e, 0x100, ...)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:157 +0x234
code.gitea.io/gitea/vendor/github.com/go-macaron/inject.(*injector).Invoke(0xc002967440, 0x119d960, 0x1334e40, 0x33, 0xc0004f8d00, 0xc00011c7b0, 0x2140960, 0xc0029e3480)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:135 +0x1b9
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Context).run(0xc0001b3e60)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:121 +0x79
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Context).Next(0xc0001b3e60)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:112 +0x2f
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.Logger.func1(0xc0001b3e60, 0xc00011c780)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/logger.go:52 +0x289
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.LoggerInvoker.Invoke(0x1334e20, 0xc002967460, 0x2, 0x2, 0xc00011c780, 0x16, 0x7fe3a5ef5598, 0xc001e40708, 0xc0029e3730)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/logger.go:40 +0x74
code.gitea.io/gitea/vendor/github.com/go-macaron/inject.(*injector).fastInvoke(0xc002967440, 0x7fe3a5ef5598, 0x1334e20, 0x147c120, 0x119d960, 0x2, 0xc0029e3788, 0x4176c2, 0xc001e40840, 0x160, ...)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:157 +0x234
code.gitea.io/gitea/vendor/github.com/go-macaron/inject.(*injector).Invoke(0xc002967440, 0x119d960, 0x1334e20, 0xc001e40920, 0xc0002a19d0, 0x70, 0xc002967440, 0xc0029e37b8)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:135 +0x1b9
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Context).run(0xc0001b3e60)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:121 +0x79
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Router).Handle.func1(0x145d560, 0xc003f7ae00, 0xc000c6c400, 0xc00207f7d0)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/router.go:187 +0x272
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Router).ServeHTTP(0xc003014960, 0x145d560, 0xc003f7ae00, 0xc000c6c400)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/router.go:303 +0x1a6
code.gitea.io/gitea/vendor/gopkg.in/macaron%2ev1.(*Macaron).ServeHTTP(0xc00308e180, 0x145d560, 0xc003f7ae00, 0xc000c6c400)
    /home/typeless/devel/go/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/macaron.go:220 +0x12a
code.gitea.io/gitea/integrations.MakeRequest(0x1475780, 0xc000c8f900, 0xc000c6c400, 0x12e, 0x8)
    /home/typeless/devel/go/src/code.gitea.io/gitea/integrations/integration_test.go:284 +0xd1
code.gitea.io/gitea/integrations.(*TestSession).MakeRequest(0xc003aea0f0, 0x1475780, 0xc000c8f900, 0xc000c6c400, 0x12e, 0xc)
    /home/typeless/devel/go/src/code.gitea.io/gitea/integrations/integration_test.go:171 +0x17d
code.gitea.io/gitea/integrations.testRepoFork(0xc000c8f900, 0xc003aea0f0, 0x12d4e6d, 0x5, 0x12d4c38, 0x5, 0x12d4e72, 0x5, 0x12d4c38, 0x5, ...)
    /home/typeless/devel/go/src/code.gitea.io/gitea/integrations/repo_fork_test.go:47 +0xa56
code.gitea.io/gitea/integrations.TestRepoForkToOrg(0xc000c8f900)
    /home/typeless/devel/go/src/code.gitea.io/gitea/integrations/repo_fork_test.go:65 +0xdd
testing.tRunner(0xc000c8f900, 0x1332418)
    /home/typeless/tools/go/src/testing/testing.go:850 +0xbf
created by testing.(*T).Run
    /home/typeless/tools/go/src/testing/testing.go:901 +0x357
lunny commented 5 years ago

@typeless updated, please try again.

typeless commented 5 years ago

@lunny It passes all tests now. 👍

typeless commented 5 years ago

@lunny

Here is another change I made to make the tests pass. But I am not sure it is correct or makes sense at all.

diff --git a/models/test_fixtures.go b/models/test_fixtures.go
index d7f59ec3b..4e1004795 100644
--- a/models/test_fixtures.go
+++ b/models/test_fixtures.go
@@ -19,5 +19,12 @@ func InitFixtures(helper testfixtures.Helper, dir string) (err error) {

 // LoadFixtures load fixtures for a test database
 func LoadFixtures() error {
-       return fixtures.Load()
+       var err error
+       for i := 0; i < 5; i++ {
+               err = fixtures.Load()
+               if err == nil {
+                       break
+               }
+       }
+       return err
 }

Without this change, the transaction could fail and turn to ROLLBACK. The exact cause of the failure is that sqlite3_unlock_notify could return SQLITE_LOCKED in certain circumstances as mentioned in https://sqlite.org/unlock_notify.html. It seems this should be addressed in application code anyway. If that's correct, I can submit a PR.

lunny commented 5 years ago

@typeless please send another PR.

typeless commented 5 years ago

@lunny https://github.com/go-gitea/gitea/pull/5125