8000 Fix SingularTable race condition by emirb · Pull Request #2396 · go-gorm/gorm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix SingularTable race condition #2396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 14, 2019
Merged

Fix SingularTable race condition #2396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5959487
Fix unsafe concurrent SingularTable method call
emirb Apr 14, 2019
b492734
gofmt
emirb Apr 14, 2019
b923e78
Verbose go get output
emirb Apr 14, 2019
96d52f2
Use RWMutex
emirb Apr 14, 2019
cd0f3ae
Run tests with race detector
emirb Apr 14, 2019
ef9d207
Run tests with race detector
emirb Apr 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

// DB contains information for current db connection
type DB struct {
sync.RWMutex
Value interface{}
Error error
RowsAffected int64
Expand Down Expand Up @@ -170,7 +171,8 @@ func (s *DB) HasBlockGlobalUpdate() bool {

// SingularTable use singular table by default
func (s *DB) SingularTable(enable bool) {
modelStructsMap = sync.Map{}
s.parent.Lock()
defer s.parent.Unlock()
s.parent.singularTable = enable
}

Expand Down
33 changes: 29 additions & 4 deletions main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"reflect"
"strconv"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -277,6 +278,30 @@ func TestTableName(t *testing.T) {
DB.SingularTable(false)
}

func TestTableNameConcurrently(t *testing.T) {
DB := DB.Model("")
if DB.NewScope(Order{}).TableName() != "orders" {
t.Errorf("Order's table name should be orders")
}

var wg sync.WaitGroup
wg.Add(10)

for i := 1; i <= 10; i++ {
go func(db *gorm.DB) {
DB.SingularTable(true)
wg.Done()
}(DB)
}
wg.Wait()

if DB.NewScope(Order{}).TableName() != "order" {
t.Errorf("Order's singular table name should be order")
}

DB.SingularTable(false)
}

func TestNullValues(t *testing.T) {
DB.DropTable(&NullValue{})
DB.AutoMigrate(&NullValue{})
Expand Down Expand Up @@ -1066,12 +1091,12 @@ func TestCountWithHaving(t *testing.T) {

DB.Create(getPreparedUser("user1", "pluck_user"))
DB.Create(getPreparedUser("user2", "pluck_user"))
user3:=getPreparedUser("user3", "pluck_user")
user3.Languages=[]Language{}
user3 := getPreparedUser("user3", "pluck_user")
user3.Languages = []Language{}
DB.Create(user3)

var count int
err:=db.Model(User{}).Select("users.id").
err := db.Model(User{}).Select("users.id").
Joins("LEFT JOIN user_languages ON user_languages.user_id = users.id").
Joins("LEFT JOIN languages ON user_languages.language_id = languages.id").
Group("users.id").Having("COUNT(languages.id) > 1").Count(&count).Error
Expand All @@ -1080,7 +1105,7 @@ func TestCountWithHaving(t *testing.T) {
t.Error("Unexpected error on query count with having")
}

if count!=2{
if count != 2 {
t.Error("Unexpected result on query count with having")
}
}
Expand Down
17 changes: 15 additions & 2 deletions model_struct.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ func (s *ModelStruct) TableName(db *DB) string {
s.defaultTableName = tabler.TableName()
} else {
tableName := ToTableName(s.ModelType.Name())
db.parent.RLock()
if db == nil || (db.parent != nil && !db.parent.singularTable) {
tableName = inflection.Plural(tableName)
}
db.parent.RUnlock()
s.defaultTableName = tableName
}
}
Expand Down Expand Up @@ -163,7 +165,18 @@ func (scope *Scope) GetModelStruct() *ModelStruct {
}

// Get Cached model struct
if value, ok := modelStructsMap.Load(reflectType); ok && value != nil {
isSingularTable := false
if scope.db != nil && scope.db.parent != nil {
scope.db.parent.RLock()
isSingularTable = scope.db.parent.singularTable
scope.db.parent.RUnlock()
}

hashKey := struct {
singularTable bool
reflectType reflect.Type
}{isSingularTable, reflectType}
if value, ok := modelStructsMap.Load(hashKey); ok && value != nil {
return value.(*ModelStruct)
}

Expand Down Expand Up @@ -612,7 +625,7 @@ func (scope *Scope) GetModelStruct() *ModelStruct {
}
}

modelStructsMap.Store(reflectType, &modelStruct)
modelStructsMap.Store(hashKey, &modelStruct)

return &modelStruct
}
Expand Down
2 changes: 1 addition & 1 deletion preload_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1677,7 +1677,7 @@ func TestPreloadManyToManyCallbacks(t *testing.T) {
lvl := Level1{
Name: "l1",
Level2s: []Level2{
Level2{Name: "l2-1"}, Level2{Name: "l2-2"},
{Name: "l2-1"}, {Name: "l2-2"},
},
}
DB.Save(&lvl)
Expand Down
24 changes: 12 additions & 12 deletions wercker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ build:
code: |
cd $WERCKER_SOURCE_DIR
go version
go get -t ./...
go get -t -v ./...

# Build the project
- script:
Expand All @@ -95,54 +95,54 @@ build:
- script:
name: test sqlite
code: |
go test ./...
go test -race -v ./...

- script:
name: test mariadb
code: |
GORM_DIALECT=mysql GORM_DSN="gorm:gorm@tcp(mariadb:3306)/gorm?charset=utf8&parseTime=True" go test ./...
GORM_DIALECT=mysql GORM_DSN="gorm:gorm@tcp(mariadb:3306)/gorm?charset=utf8&parseTime=True" go test -race ./...

- script:
name: test mysql5.7
code: |
GORM_DIALECT=mysql GORM_DSN="gorm:gorm@tcp(mysql57:3306)/gorm?charset=utf8&parseTime=True" go test ./...
GORM_DIALECT=mysql GORM_DSN="gorm:gorm@tcp(mysql57:3306)/gorm?charset=utf8&parseTime=True" go test -race ./...

- script:
name: test mysql5.6
code: |
GORM_DIALECT=mysql GORM_DSN="gorm:gorm@tcp(mysql56:3306)/gorm?charset=utf8&parseTime=True" go test ./...
GORM_DIALECT=mysql GORM_DSN="gorm:gorm@tcp(mysql56:3306)/gorm?charset=utf8&parseTime=True" go test -race ./...

- script:
name: test mysql5.5
code: |
GORM_DIALECT=mysql GORM_DSN="gorm:gorm@tcp(mysql55:3306)/gorm?charset=utf8&parseTime=True" go test ./...
GORM_DIALECT=mysql GORM_DSN="gorm:gorm@tcp(mysql55:3306)/gorm?charset=utf8&parseTime=True" go test -race ./...

- script:
name: test postgres
code: |
GORM_DIALECT=postgres GORM_DSN="host=postgres user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test ./...
GORM_DIALECT=postgres GORM_DSN="host=postgres user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test -race ./...

- script:
814C name: test postgres96
code: |
GORM_DIALECT=postgres GORM_DSN="host=postgres96 user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test ./...
GORM_DIALECT=postgres GORM_DSN="host=postgres96 user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test -race ./...

- script:
name: test postgres95
code: |
GORM_DIALECT=postgres GORM_DSN="host=postgres95 user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test ./...
GORM_DIALECT=postgres GORM_DSN="host=postgres95 user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test -race ./...

- script:
name: test postgres94
code: |
GORM_DIALECT=postgres GORM_DSN="host=postgres94 user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test ./...
GORM_DIALECT=postgres GORM_DSN="host=postgres94 user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test -race ./...

- script:
name: test postgres93
code: |
GORM_DIALECT=postgres GORM_DSN="host=postgres93 user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test ./...
GORM_DIALECT=postgres GORM_DSN="host=postgres93 user=gorm password=gorm DB.name=gorm port=5432 sslmode=disable" go test -race ./...

- script:
name: test mssql
code: |
GORM_DIALECT=mssql GORM_DSN="sqlserver://gorm:LoremIpsum86@mssql:1433?database=gorm" go test ./...
GORM_DIALECT=mssql GORM_DSN="sqlserver://gorm:LoremIpsum86@mssql:1433?database=gorm" go test -race ./...
0