Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixed windows disk package leaks #1501

Merged
merged 1 commit into from Aug 11, 2023
Merged

fix: fixed windows disk package leaks #1501

merged 1 commit into from Aug 11, 2023

Conversation

ozanh
Copy link
Contributor

@ozanh ozanh commented Aug 10, 2023

  • fixed goroutine leak in PartitionsWithContext
  • closed registry handle in init

- fixed goroutine leak in PartitionsWithContext
- closed registry handle in init
Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

Let me check the intent of this PR: is the intent to change the quitChan to wait for the quitChan to close on defer with select in order to prevent goroutine leaks?

If so, then indeed the goroutine continues to remain in the current implementation, and this PR seems to solve that.

@ozanh
Copy link
Contributor Author

ozanh commented Aug 11, 2023

Hi @shirou
When the context is done before or during PartitionsWithContext, the goroutine will still be alive because it cannot send to the channel. I intended to prevent this kind of leak scenarios. Also, I used a quit channel explicitly instead of using context's Done channel in the gouroutine to make it more robust.

It could be resolved using a buffered channel as well however my fix seemed to be an idiomatic approach with channels.

Lastly, all parameter is not used in PartitionsWithContext. I don't know the background of this intention but if it seems to be a bug I can also create another PR for that.

Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your explanation. I understood your intention and I agree with you. Thank you very much.

As for the all argument, we don't use it in the Windows implementation, but we use it on other platforms such as Linux.

@shirou shirou merged commit 8ecc0c6 into shirou:master Aug 11, 2023
19 checks passed
dave-gray101 pushed a commit to mudler/LocalAI that referenced this pull request Sep 1, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/shirou/gopsutil/v3](https://togithub.com/shirou/gopsutil)
| require | patch | `v3.23.7` -> `v3.23.8` |

---

### Release Notes

<details>
<summary>shirou/gopsutil (github.com/shirou/gopsutil/v3)</summary>

###
[`v3.23.8`](https://togithub.com/shirou/gopsutil/releases/tag/v3.23.8)

[Compare
Source](https://togithub.com/shirou/gopsutil/compare/v3.23.7...v3.23.8)

<!-- Release notes generated using configuration in .github/release.yml
at v3.23.8 -->

#### What's Changed

[#&#8203;1514](https://togithub.com/shirou/gopsutil/issues/1514)
improves `Processes()` performance 6% or more. Thank you
[@&#8203;atoulme](https://togithub.com/atoulme) !

##### cpu

- Enable setting of vendor and related information for all Power
versions by [@&#8203;kishen-v](https://togithub.com/kishen-v) in
[shirou/gopsutil#1495
- chore: change CIRCLECI environment variable to CI. by
[@&#8203;shirou](https://togithub.com/shirou) in
[shirou/gopsutil#1518

##### disk

- fix: fixed windows disk package leaks by
[@&#8203;ozanh](https://togithub.com/ozanh) in
[shirou/gopsutil#1501
- fix IOCounters() SerialNumber enumeration by
[@&#8203;gdvalle](https://togithub.com/gdvalle) in
[shirou/gopsutil#1508

##### host

- \[host]\[linux]: remove double quote from lsb release info by
[@&#8203;shirou](https://togithub.com/shirou) in
[shirou/gopsutil#1504

##### mem

- mem: linux: fix vmstat field names by
[@&#8203;chouquette](https://togithub.com/chouquette) in
[shirou/gopsutil#1498

##### process

- Fix Processes() calls with many cores by
[@&#8203;atoulme](https://togithub.com/atoulme) in
[shirou/gopsutil#1514

#### New Contributors

- [@&#8203;kishen-v](https://togithub.com/kishen-v) made their first
contribution in
[shirou/gopsutil#1495
- [@&#8203;chouquette](https://togithub.com/chouquette) made their first
contribution in
[shirou/gopsutil#1498
- [@&#8203;ozanh](https://togithub.com/ozanh) made their first
contribution in
[shirou/gopsutil#1501
- [@&#8203;gdvalle](https://togithub.com/gdvalle) made their first
contribution in
[shirou/gopsutil#1508

**Full Changelog**:
shirou/gopsutil@v3.23.7...v3.23.8

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/go-skynet/LocalAI).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi42OC4xIiwidXBkYXRlZEluVmVyIjoiMzYuNjguMSIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants