ronantakizawa/codereview-bench huggingface dataset:
Repository: syncthing/syncthing
File: lib/connections/metrics.go (Go)

Code before:
// Copyright (C) 2024 The Syncthing Authors.
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.

package connections

import (
	"github.com/prometheus/client_golang/prometheus"
	"github.com/prometheus/client_golang/prometheus/promauto"
)

var (
	metricDeviceActiveConnections = promauto.NewGaugeVec(prometheus.GaugeOpts{
		Namespace: "syncthing",
		Subsystem: "connections",
		Name:      "active",
		Help:      "Amount of currently active connections, per device. If value is 0, the device is disconnected.",
	}, []string{"device"})
)

func registerDeviceMetrics(deviceID string) {
	// Register metrics for this device, so that counters & gauges are present even
	// when zero.
	metricDeviceActiveConnections.WithLabelValues(deviceID)
}


Review comment (suggestion):
```suggestion
		Help:      "Number of currently active connections, per device. If value is 0, the device is disconnected.",
```

Code after:
// Copyright (C) 2024 The Syncthing Authors.
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.

package connections

import (
	"github.com/prometheus/client_golang/prometheus"
	"github.com/prometheus/client_golang/prometheus/promauto"
)

var (
	metricDeviceActiveConnections = promauto.NewGaugeVec(prometheus.GaugeOpts{
		Namespace: "syncthing",
		Subsystem: "connections",
		Name:      "active",
		Help:      "Number of currently active connections, per device. If value is 0, the device is disconnected.",
	}, []string{"device"})
)

func registerDeviceMetrics(deviceID string) {
	// Register metrics for this device, so that counters & gauges are present even
	// when zero.
	metricDeviceActiveConnections.WithLabelValues(deviceID)
}
Repository: syncthing/syncthing
File: lib/api/api.go (Go)

Code before:
	}
}

func (*service) getLang(w http.ResponseWriter, r *http.Request) {
	lang := r.Header.Get("Accept-Language")
	var langs = make(map[string]float64)
	for _, l := range strings.Split(lang, ",") {
		parts := strings.SplitN(l, ";", 2)
		code := strings.ToLower(strings.TrimSpace(parts[0]))
		langs[code] = 1.0
		if len(parts) < 2 {
			continue
		}
		weight := strings.ToLower(strings.TrimSpace(parts[1]))
		if weight[:2] == "q=" {
			if q, err := strconv.ParseFloat(weight[2:], 32); err != nil {
				// Completely dismiss entries with invalid weight
				delete(langs, code)
			} else {
				langs[code] = q
			}
		}
	}
	var orderedLangs = make([]string, 0, len(langs))
	for code := range langs {
		orderedLangs = append(orderedLangs, code)
	}
	// Reorder by descending q value
	sort.SliceStable(orderedLangs, func(i, j int) bool {
		return langs[orderedLangs[i]] > langs[orderedLangs[j]]
	})
	sendJSON(w, orderedLangs)
}

func (s *service) postSystemUpgrade(w http.ResponseWriter, _ *http.Request) {
	opts := s.cfg.Options()
	rel, err := upgrade.LatestRelease(opts.ReleasesURL, build.Version, opts.UpgradeToPreReleases)
	if err != nil {
		httpError(w, err)
		return
	}

	if upgrade.CompareVersions(rel.Tag, build.Version) > upgrade.Equal {
		err = upgrade.To(rel)
		if err != nil {
			l.Warnln("upgrading:", err)
			http.Error(w, err.Error(), http.StatusInternalServerError)
			return
		}

		s.flushResponse(`{"ok": "restarting"}`, w)

Review comment (suggestion):
could add them in the loop above, doesn't really matter much but one loop that defaults the weight to 1.0, tries to parse a weight and skip it it's invalid, then adds to the map and list, would be slightly clearer to me.

Code after:
	}
}

func (*service) getLang(w http.ResponseWriter, r *http.Request) {
	lang := r.Header.Get("Accept-Language")
	var weights = make(map[string]float64)
	for _, l := range strings.Split(lang, ",") {
		parts := strings.SplitN(l, ";", 2)
		code := strings.ToLower(strings.TrimSpace(parts[0]))
		weights[code] = 1.0
		if len(parts) < 2 {
			continue
		}
		weight := strings.ToLower(strings.TrimSpace(parts[1]))
		if !strings.HasPrefix(weight, "q=") {
			continue
		}
		if q, err := strconv.ParseFloat(weight[2:], 32); err != nil {
			// Completely dismiss entries with invalid weight
			delete(weights, code)
		} else {
			weights[code] = q
		}
	}
	var langs = make([]string, 0, len(weights))
	for code := range weights {
		langs = append(langs, code)
	}
	// Reorder by descending q value
	sort.SliceStable(langs, func(i, j int) bool {
		return weights[langs[i]] > weights[langs[j]]
	})
	sendJSON(w, langs)
}

func (s *service) postSystemUpgrade(w http.ResponseWriter, _ *http.Request) {
	opts := s.cfg.Options()
	rel, err := upgrade.LatestRelease(opts.ReleasesURL, build.Version, opts.UpgradeToPreReleases)
	if err != nil {
		httpError(w, err)
		return
	}

	if upgrade.CompareVersions(rel.Tag, build.Version) > upgrade.Equal {
		err = upgrade.To(rel)
		if err != nil {
			l.Warnln("upgrading:", err)
			http.Error(w, err.Error(), http.StatusInternalServerError)
			return
		}
Repository: syncthing/syncthing
File: lib/fs/casefs.go (Go)

Code before:
func (f *caseFilesystem) checkCaseExisting(name string) error {
	realName, err := f.realCase(name)
	if IsNotExist(err) {
		// It did exist just before -> cache is outdated, try again
		f.dropCache()
		realName, err = f.realCase(name)
	}
	if err != nil {
		return err
	}
	// We normalize the normalization (hah!) of the strings before
	// comparing, as we don't want to treat a normalization difference as a
	// case conflict.
	if norm.NFC.String(realName) != norm.NFC.String(name) {
		return &ErrCaseConflict{name, realName}
	}
	return nil
}

type defaultRealCaser struct {
	cache caseCache
	fs    Filesystem
	mut   sync.Mutex
}

type caseCache = *lru.TwoQueueCache[string, *caseNode]

func newCaseCache() caseCache {
	cache, err := lru.New2Q[string, *caseNode](caseCacheItemLimit)
	// New2Q only errors if given invalid parameters, which we don't.
	if err != nil {
		panic(err)
	}
	return cache
}

func (r *defaultRealCaser) realCase(name string) (string, error) {
	realName := "."
	if name == realName {
		return realName, nil
	}

	for _, comp := range PathComponents(name) {
		node := r.getExpireAdd(realName)

		if node.err != nil {
			return "", node.err
		}

		// Try to find a direct or case match
		if _, ok := node.children[comp]; !ok {

Review comment (performance):
This confused me up until reading this line, because the `map[fskey]caseCache` looks like a map of something not-pointer-shaped, and yet here we are with a pointer. I think I'd prefer the alias to not include the pointer star, and have that be visible at the places it's used instead...

Code after:
func (f *caseFilesystem) checkCaseExisting(name string) error {
	realName, err := f.realCase(name)
	if IsNotExist(err) {
		// It did exist just before -> cache is outdated, try again
		f.dropCache()
		realName, err = f.realCase(name)
	}
	if err != nil {
		return err
	}
	// We normalize the normalization (hah!) of the strings before
	// comparing, as we don't want to treat a normalization difference as a
	// case conflict.
	if norm.NFC.String(realName) != norm.NFC.String(name) {
		return &ErrCaseConflict{name, realName}
	}
	return nil
}

type defaultRealCaser struct {
	cache *caseCache
	fs    Filesystem
	mut   sync.Mutex
}

type caseCache = lru.TwoQueueCache[string, *caseNode]

func newCaseCache() *caseCache {
	cache, err := lru.New2Q[string, *caseNode](caseCacheItemLimit)
	// New2Q only errors if given invalid parameters, which we don't.
	if err != nil {
		panic(err)
	}
	return cache
}

func (r *defaultRealCaser) realCase(name string) (string, error) {
	realName := "."
	if name == realName {
		return realName, nil
	}

	for _, comp := range PathComponents(name) {
		node := r.getExpireAdd(realName)

		if node.err != nil {
			return "", node.err
		}

		// Try to find a direct or case match
		if _, ok := node.children[comp]; !ok {
Repository: syncthing/syncthing
File: lib/fs/filesystem.go (Go)

Code before:
// IsPathSeparator is the equivalent of os.IsPathSeparator
var IsPathSeparator = os.IsPathSeparator

func NewFilesystem(fsType FilesystemType, uri string, opts ...Option) Filesystem {
	var caseOpt Option
	var mtimeOpt Option
	i := 0
	for _, opt := range opts {
		if caseOpt != nil && mtimeOpt != nil {
			break
		}
		switch opt.(type) {
		case *OptionDetectCaseConflicts:
			caseOpt = opt
		case *optionMtime:
			mtimeOpt = opt
		default:
			opts[i] = opt
			i++
		}
	}
	opts = opts[:i]

	// Construct file system using the registered factory function
	var fs Filesystem
	if fsFactory, ok := filesystemFactories[fsType]; ok {
		var err error
		fs, err = fsFactory(fsType, uri, opts...)
		if err != nil {
			fs = &errorFilesystem{
				fsType: fsType,
				uri:    uri,
				err:    err,
			}
		}
	} else {
		fs = &errorFilesystem{
			fsType: fsType,
			uri:    uri,
			err:    fmt.Errorf("File system type '%s' not recognized", fsType),
		}
	}

	// mtime handling should happen inside walking, as filesystem calls while
	// walking should be mtime-resolved too
	if mtimeOpt != nil {
		fs = mtimeOpt.apply(fs)
	}

	fs = &metricsFS{next: fs}


Review comment (suggestion):
I think this thing could be slightly simplified by something like

```
var fs Filesystem
var err error
if fact, ok := filesystemFactories[fsType]; ok {
  fs, err = fsFactory(fsType, uri, opts...)
} else {
  err = fmt.Errorf("File system type '%s' not recognized", fsType)
}
if err != nil {
  fs = &errorFilesystem{
    fsType: fsType,
    uri:    uri,
    err:    err,
  }
}
```

oh and we must also lock the mutex here

Code after:
// IsPathSeparator is the equivalent of os.IsPathSeparator
var IsPathSeparator = os.IsPathSeparator

func NewFilesystem(fsType FilesystemType, uri string, opts ...Option) Filesystem {
	var caseOpt Option
	var mtimeOpt Option
	i := 0
	for _, opt := range opts {
		if caseOpt != nil && mtimeOpt != nil {
			break
		}
		switch opt.(type) {
		case *OptionDetectCaseConflicts:
			caseOpt = opt
		case *optionMtime:
			mtimeOpt = opt
		default:
			opts[i] = opt
			i++
		}
	}
	opts = opts[:i]

	// Construct file system using the registered factory function
	var fs Filesystem
	var err error
	filesystemFactoriesMutex.Lock()
	fsFactory, factoryFound := filesystemFactories[fsType]
	filesystemFactoriesMutex.Unlock()
	if factoryFound {
		fs, err = fsFactory(uri, opts...)
	} else {
		err = fmt.Errorf("File system type '%s' not recognized", fsType)
	}

	if err != nil {
		fs = &errorFilesystem{
			fsType: fsType,
			uri:    uri,
			err:    err,
		}
	}

	// mtime handling should happen inside walking, as filesystem calls while
	// walking should be mtime-resolved too
	if mtimeOpt != nil {
		fs = mtimeOpt.apply(fs)
	}

	fs = &metricsFS{next: fs}
Repository: syncthing/syncthing
File: lib/config/config_test.go (Go)

Code before:

		_, err = testFs.Stat(DefaultMarkerName)
		if i < 6 && err != nil {
			t.Fatal(err)
		} else if i >= 6 && err == nil {
			t.Fatal("Unexpected file")
		}

		cfg := wr.Wrapper.(*wrapper).cfg

		fc := FolderConfiguration{}
		structutil.SetDefaults(&fc)
		fc.ID = "test"
		fc.FilesystemType = FilesystemTypeBasic
		fc.Path = "testdata"
		fc.Devices = []FolderDeviceConfiguration{{DeviceID: device1}, {DeviceID: device4}}
		fc.Type = FolderTypeSendOnly
		fc.RescanIntervalS = 600
		fc.FSWatcherEnabled = false
		fc.FSWatcherDelayS = 10
		fc.Copiers = 0
		fc.Hashers = 0
		fc.AutoNormalize = true
		fc.MinDiskFree = Size{1, "%"}
		fc.MaxConflicts = -1
		fc.Versioning.CleanupIntervalS = 1
		fc.JunctionsAsDirs = true
		fc.MaxConcurrentWrites = maxConcurrentWritesDefault
		fc.FSWatcherEnabled = true
		fc.Versioning.CleanupIntervalS = 3600
		fc.Versioning.Params = map[string]string{}
		fc.XattrFilter.Entries = []XattrFilterEntry{}

		expectedDevices := []DeviceConfiguration{
			{
				DeviceID:        device1,
				Name:            "node one",
				Addresses:       []string{"tcp://a"},
				Compression:     CompressionMetadata,
				AllowedNetworks: []string{},
				IgnoredFolders:  []ObservedFolder{},
			},
			{
				DeviceID:        device4,
				Name:            "node two",
				Addresses:       []string{"tcp://b"},
				Compression:     CompressionMetadata,
				AllowedNetworks: []string{},
				IgnoredFolders:  []ObservedFolder{},
			},
		}

Review comment (refactor):
Is this on purpose? There is also `fc.Versioning.CleanupIntervalS = 3600` a few lines below. `fc.FSWatcherEnabled` also seems duplicated.

Code after:
		_, err = testFs.Stat(DefaultMarkerName)
		if i < 6 && err != nil {
			t.Fatal(err)
		} else if i >= 6 && err == nil {
			t.Fatal("Unexpected file")
		}

		cfg := wr.Wrapper.(*wrapper).cfg

		expectedFolders := []FolderConfiguration{
			{
				ID:               "test",
				FilesystemType:   FilesystemTypeBasic,
				Path:             "testdata",
				Devices:          []FolderDeviceConfiguration{{DeviceID: device1}, {DeviceID: device4}},
				Type:             FolderTypeSendOnly,
				RescanIntervalS:  600,
				FSWatcherEnabled: true,
				FSWatcherDelayS:  10,
				Copiers:          0,
				Hashers:          0,
				AutoNormalize:    true,
				MinDiskFree:      Size{1, "%"},
				MaxConflicts:     -1,
				Versioning: VersioningConfiguration{
					CleanupIntervalS: 3600,
					FSType:           FilesystemTypeBasic,
					Params:           map[string]string{},
				},
				WeakHashThresholdPct: 25,
				MarkerName:           DefaultMarkerName,
				JunctionsAsDirs:      true,
				MaxConcurrentWrites:  maxConcurrentWritesDefault,
				XattrFilter: XattrFilter{
					MaxSingleEntrySize: 1024,
					MaxTotalSize:       4096,
					Entries:            []XattrFilterEntry{},
				},
			},
		}

		expectedDevices := []DeviceConfiguration{
			{
				DeviceID:        device1,
				Name:            "node one",
				Addresses:       []string{"tcp://a"},
				Compression:     CompressionMetadata,
				AllowedNetworks: []string{},
				IgnoredFolders:  []ObservedFolder{},
			},
			{
Repository: syncthing/syncthing
File: lib/api/api_auth.go (Go)

Code before:

func emitLoginAttempt(success bool, username, address string, evLogger events.Logger) {
	evLogger.Log(events.LoginAttempt, map[string]interface{}{
		"success":       success,
		"username":      username,
		"remoteAddress": address,
	})
	if !success {
		l.Infof("Wrong credentials supplied during API authorization from %s", address)
	}
}

func antiBruteForceSleep() {
	time.Sleep(time.Duration(rand.Intn(100)+100) * time.Millisecond)
}

func unauthorized(w http.ResponseWriter, shortID string) {
	w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Basic realm="Authorization Required (%s)"`, shortID))
	http.Error(w, "Not Authorized", http.StatusUnauthorized)
}

func forbidden(w http.ResponseWriter) {
	http.Error(w, "Forbidden", http.StatusForbidden)
}

func isNoAuthPath(path string, promMetricsNoAuth bool) bool {
	// Local variable instead of module var to prevent accidental mutation
	noAuthPaths := []string{
		"/",
		"/index.html",
		"/modal.html",
		"/rest/svc/lang", // Required to load language settings on login page
	}

	if promMetricsNoAuth { // 'Prom Metrics No Auth' GUI option
		noAuthPaths = append(noAuthPaths, "/metrics")
	}

	// Local variable instead of module var to prevent accidental mutation
	noAuthPrefixes := []string{
		// Static assets
		"/assets/",
		"/syncthing/",
		"/vendor/",
		"/theme-assets/", // This leaks information from config, but probably not sensitive

		// No-auth API endpoints
		"/rest/noauth",
	}

	return slices.Contains(noAuthPaths, path) ||

Review comment (security):
```suggestion
func isNoAuthPath(path string, metricsWithoutAuth bool) bool {
```

Code after:

func emitLoginAttempt(success bool, username, address string, evLogger events.Logger) {
	evLogger.Log(events.LoginAttempt, map[string]interface{}{
		"success":       success,
		"username":      username,
		"remoteAddress": address,
	})
	if !success {
		l.Infof("Wrong credentials supplied during API authorization from %s", address)
	}
}

func antiBruteForceSleep() {
	time.Sleep(time.Duration(rand.Intn(100)+100) * time.Millisecond)
}

func unauthorized(w http.ResponseWriter, shortID string) {
	w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Basic realm="Authorization Required (%s)"`, shortID))
	http.Error(w, "Not Authorized", http.StatusUnauthorized)
}

func forbidden(w http.ResponseWriter) {
	http.Error(w, "Forbidden", http.StatusForbidden)
}

func isNoAuthPath(path string, metricsWithoutAuth bool) bool {
	// Local variable instead of module var to prevent accidental mutation
	noAuthPaths := []string{
		"/",
		"/index.html",
		"/modal.html",
		"/rest/svc/lang", // Required to load language settings on login page
	}

	if metricsWithoutAuth {
		noAuthPaths = append(noAuthPaths, "/metrics")
	}

	// Local variable instead of module var to prevent accidental mutation
	noAuthPrefixes := []string{
		// Static assets
		"/assets/",
		"/syncthing/",
		"/vendor/",
		"/theme-assets/", // This leaks information from config, but probably not sensitive

		// No-auth API endpoints
		"/rest/noauth",
	}

	return slices.Contains(noAuthPaths, path) ||
Repository: syncthing/syncthing
File: lib/versioner/util.go (Go)

Code before:
	ver := tagger(file, now.Format(TimeFormat))
	dst := filepath.Join(inFolderPath, ver)
	l.Debugln("archiving", filePath, "moving to", dst)
	err = osutil.RenameOrCopy(method, srcFs, dstFs, filePath, dst)

	mtime := info.ModTime()
	// If it's a trashcan versioner type thing, then it does not have version time in the name
	// so use mtime for that.
	if ver == file {
		mtime = now
	}

	_ = dstFs.Chtimes(dst, mtime, mtime)

	return err
}

func dupDirTree(srcFs, dstFs fs.Filesystem, folderPath string) error {
	// Return early if the folder already exists.
	_, err := dstFs.Stat(folderPath)
	if err == nil || !fs.IsNotExist(err) {
		return err
	}
	i := 0
	hadParent := true
	for i < len(folderPath) {
		if os.IsPathSeparator(folderPath[i]) {
			// If the parent folder didn't exist, then this folder doesn't exist
			// so we can skip the check
			if hadParent {
				_, err := dstFs.Stat(folderPath[:i])
				if err == nil {
					i++
					continue
				}
				if !fs.IsNotExist(err) {
					return err
				}
			}
			hadParent = false
			err := dupDirWithPerms(srcFs, dstFs, folderPath[:i])
			if err != nil {
				return err
			}
		}
		i++
	}
	return dupDirWithPerms(srcFs, dstFs, folderPath)
}

func dupDirWithPerms(srcFs, dstFs fs.Filesystem, folderPath string) error {

Review comment (suggestion):
`for i := range folderPath` should be simpler here I think, avoids `i++` in the loop and avoids looking at partial code points.

(I had to think through the subtleties of string indexing and bytes vs code points for the things that happen here, but I think it's fine in both your version and in the above change.)

Code after:
	ver := tagger(file, now.Format(TimeFormat))
	dst := filepath.Join(inFolderPath, ver)
	l.Debugln("archiving", filePath, "moving to", dst)
	err = osutil.RenameOrCopy(method, srcFs, dstFs, filePath, dst)

	mtime := info.ModTime()
	// If it's a trashcan versioner type thing, then it does not have version time in the name
	// so use mtime for that.
	if ver == file {
		mtime = now
	}

	_ = dstFs.Chtimes(dst, mtime, mtime)

	return err
}

func dupDirTree(srcFs, dstFs fs.Filesystem, folderPath string) error {
	// Return early if the folder already exists.
	_, err := dstFs.Stat(folderPath)
	if err == nil || !fs.IsNotExist(err) {
		return err
	}
	hadParent := true
	for i := range folderPath {
		if os.IsPathSeparator(folderPath[i]) {
			// If the parent folder didn't exist, then this folder doesn't exist
			// so we can skip the check
			if hadParent {
				_, err := dstFs.Stat(folderPath[:i])
				if err == nil {
					continue
				}
				if !fs.IsNotExist(err) {
					return err
				}
			}
			hadParent = false
			err := dupDirWithPerms(srcFs, dstFs, folderPath[:i])
			if err != nil {
				return err
			}
		}
	}
	return dupDirWithPerms(srcFs, dstFs, folderPath)
}

func dupDirWithPerms(srcFs, dstFs fs.Filesystem, folderPath string) error {
	srcStat, err := srcFs.Stat(folderPath)
	if err != nil {
		return err
Repository: syncthing/syncthing
File: lib/protocol/protocol.go (Go)

Code before:
	defer c.startStopMut.Unlock()
	c.closeOnce.Do(func() {
		l.Debugf("close connection to %s at %s due to %v", c.deviceID.Short(), c.ConnectionInfo, err)
		if cerr := c.closer.Close(); cerr != nil {
			l.Debugf("failed to close underlying conn %s at %s %v:", c.deviceID.Short(), c.ConnectionInfo, cerr)
		}
		close(c.closed)

		c.awaitingMut.Lock()
		for i, ch := range c.awaiting {
			if ch != nil {
				close(ch)
				delete(c.awaiting, i)
			}
		}
		c.awaitingMut.Unlock()

		if !c.startTime.IsZero() {
			// Wait for the dispatcher loop to exit, if it was started to
			// begin with.
			<-c.dispatcherLoopStopped
		}

		// We don't want to call into the model while holding the
		// startStopMut, so defer it into the background.
		go c.model.Closed(err)
	})
}

// The pingSender makes sure that we've sent a message within the last
// PingSendInterval. If we already have something sent in the last
// PingSendInterval/2, we do nothing. Otherwise we send a ping message. This
// results in an effecting ping interval of somewhere between
// PingSendInterval/2 and PingSendInterval.
func (c *rawConnection) pingSender() {
	ticker := time.NewTicker(PingSendInterval / 2)
	defer ticker.Stop()

	for {
		select {
		case <-ticker.C:
			d := time.Since(c.cw.Last())
			if d < PingSendInterval/2 {
				l.Debugln(c.deviceID, "ping skipped after wr", d)
				continue
			}

			l.Debugln(c.deviceID, "ping -> after", d)
			c.ping()

		case <-c.closed:

Review comment (suggestion):
Very optional suggestion: Given how small and simple this function is, we could also unlock before this instead of deferring (moving the locking inside the once, which imo is anyway a bit more consistent).

Code after:
		c.startStopMut.Lock()

		l.Debugf("close connection to %s at %s due to %v", c.deviceID.Short(), c.ConnectionInfo, err)
		if cerr := c.closer.Close(); cerr != nil {
			l.Debugf("failed to close underlying conn %s at %s %v:", c.deviceID.Short(), c.ConnectionInfo, cerr)
		}
		close(c.closed)

		c.awaitingMut.Lock()
		for i, ch := range c.awaiting {
			if ch != nil {
				close(ch)
				delete(c.awaiting, i)
			}
		}
		c.awaitingMut.Unlock()

		if !c.startTime.IsZero() {
			// Wait for the dispatcher loop to exit, if it was started to
			// begin with.
			<-c.dispatcherLoopStopped
		}

		c.startStopMut.Unlock()

		// We don't want to call into the model while holding the
		// startStopMut.
		c.model.Closed(err)
	})
}

// The pingSender makes sure that we've sent a message within the last
// PingSendInterval. If we already have something sent in the last
// PingSendInterval/2, we do nothing. Otherwise we send a ping message. This
// results in an effecting ping interval of somewhere between
// PingSendInterval/2 and PingSendInterval.
func (c *rawConnection) pingSender() {
	ticker := time.NewTicker(PingSendInterval / 2)
	defer ticker.Stop()

	for {
		select {
		case <-ticker.C:
			d := time.Since(c.cw.Last())
			if d < PingSendInterval/2 {
				l.Debugln(c.deviceID, "ping skipped after wr", d)
				continue
			}

			l.Debugln(c.deviceID, "ping -> after", d)
			c.ping()