Concurrency: race conditions and mutex locks

[bennyscetbun]

Adding mutex on every Plan of a table to avoid race condition when using goroutine and Gorp.

[coopernurse]

Do we need a separate mutex per method on TableMap? What if we just had a single mutex and used it on each of the relevant methods

Any other opinions?

[bennyscetbun]

The idea was to avoid a single lock(in case that we ve got a lot of goroutine doing different things).
But yeah we could use your way. A single mutex would work :)

[coopernurse]

That makes sense, although the contention would likely still occur with 4 mutexes instead of one.

If we're worried about contention (and it's a valid concern I think) we may want to move the lock inside that the code that checks for empty string. For example:

func (t *TableMap) bindInsert(elem reflect.Value) (bindInstance, error) {
    plan := t.insertPlan
    if plan.query == "" {

[coopernurse]

That makes sense. Thanks.

[bennyscetbun]

I think it must be before

if plan.query == "" {

The racing condition was on plan.query ;)

And that way we avoid to run twice the creation code.

By the way thanks for your lib!

[GeertJohan]

I think we need to take a good look at this, it is very important to make sure gorp is concurrency safe!

[GeertJohan]

Superseded by #301, which uses sync.Once.