Fix context leak in tests

[munahaf]

What changed?
A marker has been added to the code. This is not an actual fix, but a marker denoting that there may be a bug.

Why?
In file: taskpoller.go, there is a function createContext that contains a cancellation function returned by context.WithTimeout. However, the cancellation function is ignored using _. I suggested that the cancellation function must be called or the new context will remain live until the parent context is cancelled. I did not propose a fix since I am not familiar with the business logic. However, the codebase appears to be following that prescribed convention in other parts.

Sponsorship and Support:
This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF)(https://openssf.org/): Project Alpha-Omega(https://alpha-omega.dev/). Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.

How did you test it?
Not reproduced.

Potential risks
Cannot ascertain.

Release notes
Appears not a release candidate.

Documentation Changes
Appears no change is needed.

[CLAassistant]

All committers have signed the CLA.

[Shaddoll]

the context will be cancelled immediately after this function returns. We should also return cancel and let caller to call cancel.
BTW, this code is only used in our integration test. It's not good but it's ok.

[adonovan]

If you run "go vet" over your code, it should point out mistakes of this kind. For example:
https://go.dev/play/p/1zlF7qFbzhY

[taylanisikdemir]

hey @adonovan, the proper fix would be to return cancel callback and change all the callers with defer cancel(). Do you want to proceed with that? Otherwise let's close this PR and we will find someone else to address it

[openrefactorymunawar]

I have prepared an updated patch. However, I am traveling now and cannot access the GitHub account used to create the pull request. I will address this in about a week.

[Groxx]

While I agree with this in general, I don't think it's true for context.Background().

The leak would be coming from the parent's list of children to propagate cancels, but a background context both does not have that list, and it has a nil done chan so this branch is taken when making a cancel-context derived from it: https://cs.opensource.google/go/go/+/refs/tags/go1.21.5:src/context/context.go;l=461

So these should be cleaned up as soon as they're garbage-collectable. Which is fine.
Though we are probably leaking the background timer itself for <90s. Which is thankfully not infinite, but seems worth cleaning up.

That said, these kinds of funcs mean we can't easily add context data (no parent-ctx arg), and this is a potential landmine if we did (because at that point it'd likely leak). So as long as we've got a clear path to cancel correctly, it seems like an improvement.

[taylanisikdemir]

thanks for addressing this!

[coveralls]

Pull Request Test Coverage Report for Build 018da900-96a5-4f49-9b26-1a6ebd52dce3

Details

• -63 of 131 (51.91%) changed or added relevant lines in 6 files are covered.
• 107 unchanged lines in 21 files lost coverage.
• Overall coverage decreased (-0.04%) to 62.745%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
host/esutils/client_v6.go	18	21	85.71%
host/taskpoller.go	41	59	69.49%
host/esutils/client_os2.go	0	21	0.0%
host/esutils/client_v7.go	0	21	0.0%

Files with Coverage Reduction	New Missed Lines	%
common/persistence/sql/common.go	2	72.73%
common/persistence/sql/sqlplugin/mysql/db.go	2	83.33%
common/persistence/sql/sqlplugin/mysql/task.go	2	73.68%
common/persistence/sql/sqlplugin/postgres/db.go	2	85.0%
common/persistence/sql/sqlplugin/postgres/task.go	2	73.4%
service/history/task/transfer_standby_task_executor.go	2	87.42%
service/matching/db.go	2	73.23%
common/log/tag/tags.go	3	50.18%
common/persistence/nosql/nosqlplugin/cassandra/workflow.go	3	59.1%
common/task/fifo_task_scheduler.go	3	84.54%

Totals
Change from base Build 018da81a-4d02-4ee6-9401-7a60db74410e:	-0.04%
Covered Lines:	92605
Relevant Lines:	147589

---

💛 - Coveralls