Skip to content

sync: delete TestCondCopy? #16229

Closed
Closed
@josharian

Description

@josharian

It appears to be testing copying a lock, which people shouldn't do.

func TestCondCopy(t *testing.T) {
    defer func() {
        err := recover()
        if err == nil || err.(string) != "sync.Cond is copied" {
            t.Fatalf("got %v, expect sync.Cond is copied", err)
        }
    }()
    c := Cond{L: &Mutex{}}
    c.Signal()
    c2 := c
    c2.Signal()
}

Vet doesn't like it:

sync/cond_test.go:254: assignment copies lock value to c2: sync.Cond contains sync.noCopy

Objections to it being deleted for 1.8?

cc @dvyukov

Part of dealing with #11041.

Activity

added this to the Go1.8 milestone on Jun 30, 2016
self-assigned this
on Jun 30, 2016
dvyukov

dvyukov commented on Jun 30, 2016

@dvyukov
Member

It appears to be testing copying a lock, which people shouldn't do.

The mutex is not copied here. Cond contains a pointer to the mutex. But Cond itself should not be copied.

The test tests a useful property. Not all use vet. Cond detects the misuse reliably at runtime. Can we move it to _novet.go or something?

josharian

josharian commented on Jun 30, 2016

@josharian
ContributorAuthor

Ok, thanks. I'll figure out a way to add an exception for it as part of #11041 -- it's not going to be the only exception.

locked and limited conversation to collaborators on Jun 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @josharian@dvyukov@gopherbot

        Issue actions

          sync: delete TestCondCopy? · Issue #16229 · golang/go