On Twitter a couple of days ago I posted this tweet which had this image:
Dave pointed out I should use the -covermode=atomic
to check for data races. That of course led to another completely separate issue where it turns out Go was able to create channels of negative sizes. I had spent the past few days puzzling over that bug, and today I finally decided to put the debugging of that on the back burner and come back to the issue of having different coverage amounts for different GOMAXPROCS
.
A big issue was that the -race
flag did not pick up any data races. Nor should it because the majority of this package does not use channels and goroutines *The idea for building this package is to get the basics down correctly first before making any guarantees on concurrency. I have a feeling that the `tensor` data structures have exposed threadsafe reads and write methods (as well as unsafe ones). And as far as I could reason about my code, there could be no data races, but I’ve been known to be wrong.
So after implementing a variation of the code that caused the channel bug above, I took Dave’s advice, and did a diff of the coverprofiles of the different GOMAXPROCS
. This is what I discovered:
With GOMAXPROCS=3
, this code was covered:
// destroyMultIterator returns any borrowed objects back to pool
func destroyMultIterator(it *MultIterator) {
if cap(it.whichBlock) > 0 {
ReturnInts(it.whichBlock)
it.whichBlock = nil
}
if cap(it.lastIndexArr) > 0 {
ReturnInts(it.lastIndexArr)
it.lastIndexArr = nil
}
if cap(it.strides) > 0 {
ReturnInts(it.strides)
it.strides = nil
}
if it.numMasked > 1 {
if cap(it.mask) > 0 {
ReturnBools(it.mask)
it.mask = nil
}
}
}
But with GOMAXPROCS=2
it wasn’t. Why?
Finalizers are a Side Effect
The destroyMultIterator
function is called when the object has fallen out of scope completely and will not be used. It’s set by a runtime.SetFinalizer
call somewhere in the creation of a *MultIterator
. Typically the finalize function is called when the garbage collection process is called. In this case, it tries not to destroy the object, and instead, put it back into a sync.Pool
.
In the runtime
docs on SetFinalizer
, it’s mentioned:
There is no guarantee that finalizers will run before a program exits, so typically they are useful only for releasing non-memory resources associated with an object during a long-running program
Now, it’s completely fine to use SetFinalizer
with regards to the iterators as they’re really just helper data structures that are transient (i.e. a non-memory resource), and depending on usecases, can be quite frequently used. However, if you do use SetFinalizer
then be aware that if you don’t specifically test the function that your finalizer calls, you could end up with uneven test coverage numbers.