-
Notifications
You must be signed in to change notification settings - Fork 0
feat(corpus): lock the ShareX TcpListener + StreamReader leaks as regressions (recall 10→12) #61
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 14 additions & 0 deletions
14
corpus/real-world/sharex-streamreader-not-disposed/after.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // AFTER (fixed): scope the reader with `using` too, so the StreamReader is disposed (not | ||
| // just the underlying stream). Disposed on every path -> silent. | ||
| using System.IO; | ||
|
|
||
| static class XmlFormatter | ||
| { | ||
| static string ToText(byte[] payload) | ||
| { | ||
| using MemoryStream ms = new MemoryStream(payload); | ||
| ms.Position = 0; | ||
| using StreamReader sReader = new StreamReader(ms); | ||
| return sReader.ReadToEnd(); | ||
| } | ||
| } |
20 changes: 20 additions & 0 deletions
20
corpus/real-world/sharex-streamreader-not-disposed/before.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // BEFORE (buggy). Reduced from ShareX @ ed2a864 — | ||
| // ShareX.HelpersLib/Helpers/Helpers.cs:854 (an XML-to-string formatter), found by mining | ||
| // (docs/notes/real-world-mining.md). | ||
| // | ||
| // A StreamReader is wrapped around a MemoryStream to read it back to a string, but the | ||
| // reader is never disposed. StreamReader is IDisposable and a SEPARATE resource from the | ||
| // (using-scoped) stream — the reader itself is left undisposed: the CA2000 / CodeQL | ||
| // cs/local-not-disposed class. Uses real System.IO types (BCL, no ref pack). | ||
| using System.IO; | ||
|
|
||
| static class XmlFormatter | ||
| { | ||
| static string ToText(byte[] payload) | ||
| { | ||
| using MemoryStream ms = new MemoryStream(payload); | ||
| ms.Position = 0; | ||
| StreamReader sReader = new StreamReader(ms); // <-- OWN001: never disposed | ||
| return sReader.ReadToEnd(); | ||
| } | ||
| } |
19 changes: 19 additions & 0 deletions
19
corpus/real-world/sharex-streamreader-not-disposed/case.own
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // OwnLang model of a real disposable leak found by mining ShareX @ ed2a864 | ||
| // (ShareX.HelpersLib/Helpers/Helpers.cs:854). A StreamReader wraps a MemoryStream to read it | ||
| // back to a string and is never disposed — StreamReader is a separate IDisposable from the | ||
| // (using-scoped) stream. Modelled as a disposable acquire with no release -> OWN001. See | ||
| // notes.md. | ||
| module Corpus | ||
| resource Reader { | ||
| acquire create | ||
| release dispose | ||
| kind "disposable" | ||
| emit_type "StreamReader" | ||
| emit_acquire "new StreamReader({args})" | ||
| emit_release "{0}.Dispose()" | ||
| } | ||
| fn ToText() { | ||
| let sReader = acquire Reader(0); // new StreamReader(ms) | ||
| // ReadToEnd() is a use | ||
| // no `release sReader;` -> never disposed (OWN001) | ||
| } |
1 change: 1 addition & 0 deletions
1
corpus/real-world/sharex-streamreader-not-disposed/expected-diagnostics.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| OWN001 |
30 changes: 30 additions & 0 deletions
30
corpus/real-world/sharex-streamreader-not-disposed/notes.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| # ShareX — `StreamReader` over a stream, never disposed | ||
|
|
||
| **Found by mining** `ShareX/ShareX` @ `ed2a864` (the WinForms re-mine — see | ||
| `docs/notes/real-world-mining.md`). Location: | ||
| `ShareX.HelpersLib/Helpers/Helpers.cs:854`. | ||
|
|
||
| ## The bug | ||
|
|
||
| ```csharp | ||
| StreamReader sReader = new StreamReader(ms); | ||
| return sReader.ReadToEnd(); | ||
| ``` | ||
|
|
||
| A `StreamReader` is created to read a `MemoryStream` back to a string and is never disposed. | ||
| `StreamReader` is `IDisposable` and a **separate** resource from the underlying stream — even | ||
| when the stream is `using`-scoped (as it is in the original), the reader itself is left | ||
| undisposed. The CA2000 / CodeQL `cs/local-not-disposed` class; low practical impact (the | ||
| reader's only unmanaged tie is the stream, already disposed), but a real undisposed | ||
| `IDisposable` a strict analyzer flags. The fix is `using` on the reader too. | ||
|
|
||
| ## What the checker says (real extractor output, `--flow-locals`) | ||
|
|
||
| ```text | ||
| Helpers.cs:854: error: [OWN001] IDisposable local 'sReader' is never disposed | ||
| (leak) [resource: disposable] | ||
| ``` | ||
|
|
||
| `acquire` is `new StreamReader(ms)`, the missing `release` is the absent `Dispose()`; | ||
| `ReadToEnd()` is a use. The local does not escape (only the returned string does), so it | ||
| stays tracked. |
16 changes: 16 additions & 0 deletions
16
corpus/real-world/sharex-tcplistener-not-disposed/after.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // AFTER (fixed): scope the listener with `using` so it is disposed (Stop() alone does not | ||
| // dispose it). Disposed on every path -> silent. | ||
| using System.Net; | ||
| using System.Net.Sockets; | ||
|
|
||
| static class PortFinder | ||
| { | ||
| static int GetRandomUnusedPort() | ||
| { | ||
| using TcpListener listener = new TcpListener(IPAddress.Loopback, 0); | ||
| listener.Start(); | ||
| int port = ((IPEndPoint)listener.LocalEndpoint).Port; | ||
| listener.Stop(); | ||
| return port; | ||
| } | ||
| } |
24 changes: 24 additions & 0 deletions
24
corpus/real-world/sharex-tcplistener-not-disposed/before.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // BEFORE (buggy). Reduced from ShareX @ ed2a864 — | ||
| // ShareX.HelpersLib/Helpers/WebHelpers.cs:191 (GetRandomUnusedPort), found by mining | ||
| // (docs/notes/real-world-mining.md). | ||
| // | ||
| // A TcpListener is started to grab a free port, then Stop()'d — but TcpListener is | ||
| // IDisposable (since .NET Core 3.0) and Stop() is NOT Dispose(): the listener's socket | ||
| // handle is not released, so the listener leaks. The CA2000 / CodeQL cs/local-not-disposed | ||
| // class. (The real method wraps the Start/return in a try/finally with the Stop in the | ||
| // finally; reduced here to the straight-line essence — the leak is the missing Dispose, | ||
| // not the control flow.) Uses real System.Net.Sockets types (BCL, no ref pack). | ||
| using System.Net; | ||
| using System.Net.Sockets; | ||
|
|
||
| static class PortFinder | ||
| { | ||
| static int GetRandomUnusedPort() | ||
| { | ||
| TcpListener listener = new TcpListener(IPAddress.Loopback, 0); // <-- OWN001: never disposed | ||
| listener.Start(); | ||
| int port = ((IPEndPoint)listener.LocalEndpoint).Port; | ||
| listener.Stop(); // Stop() releases the listen socket but is NOT Dispose() -> the listener leaks | ||
| return port; | ||
| } | ||
| } | ||
19 changes: 19 additions & 0 deletions
19
corpus/real-world/sharex-tcplistener-not-disposed/case.own
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // OwnLang model of a real disposable leak found by mining ShareX @ ed2a864 | ||
| // (ShareX.HelpersLib/Helpers/WebHelpers.cs:191, GetRandomUnusedPort). A TcpListener is | ||
| // Start()'d and Stop()'d but never disposed — Stop() is not Dispose(), and TcpListener is | ||
| // IDisposable, so the listen socket handle leaks. Modelled as a disposable acquire with no | ||
| // release -> the generic OWN001. See notes.md. | ||
| module Corpus | ||
| resource Listener { | ||
| acquire create | ||
| release dispose | ||
| kind "disposable" | ||
| emit_type "TcpListener" | ||
| emit_acquire "new TcpListener({args})" | ||
| emit_release "{0}.Dispose()" | ||
| } | ||
| fn GetRandomUnusedPort() { | ||
| let listener = acquire Listener(0); // new TcpListener(IPAddress.Loopback, 0) | ||
| // Start() / LocalEndpoint / Stop() are uses; Stop() is NOT Dispose() | ||
| // no `release listener;` -> never disposed (OWN001) | ||
| } |
1 change: 1 addition & 0 deletions
1
corpus/real-world/sharex-tcplistener-not-disposed/expected-diagnostics.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| OWN001 |
40 changes: 40 additions & 0 deletions
40
corpus/real-world/sharex-tcplistener-not-disposed/notes.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # ShareX — `TcpListener` started and stopped, never disposed | ||
|
|
||
| **Found by mining** `ShareX/ShareX` @ `ed2a864` (the WinForms re-mine — see | ||
| `docs/notes/real-world-mining.md`). Location: | ||
| `ShareX.HelpersLib/Helpers/WebHelpers.cs:191`, `GetRandomUnusedPort`. | ||
|
|
||
| ## The bug | ||
|
|
||
| ```csharp | ||
| public static int GetRandomUnusedPort() | ||
| { | ||
| TcpListener listener = new TcpListener(IPAddress.Loopback, 0); | ||
| try | ||
| { | ||
| listener.Start(); | ||
| return ((IPEndPoint)listener.LocalEndpoint).Port; | ||
| } | ||
| finally | ||
| { | ||
| listener.Stop(); // Stop() != Dispose() | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| `TcpListener` is `IDisposable` (since .NET Core 3.0). `Stop()` closes the active listen | ||
| socket but does **not** dispose the listener, so the instance is never disposed — the | ||
| canonical CA2000 / CodeQL `cs/local-not-disposed` leak. The "grab a random free port" idiom | ||
| is exactly where it slips: the developer reaches for `Stop()` and assumes it is the cleanup. | ||
| The fix is `using` (or an explicit `Dispose()`), as `after.cs` shows. | ||
|
|
||
| ## What the checker says (real extractor output, `--flow-locals`) | ||
|
|
||
| ```text | ||
| WebHelpers.cs:191: error: [OWN001] IDisposable local 'listener' is never disposed | ||
| (leak) [resource: disposable] | ||
| ``` | ||
|
|
||
| `acquire` is `new TcpListener(...)`, the missing `release` is the absent `Dispose()`; | ||
| `Start()` / `Stop()` are uses, not releases. `before.cs` reduces the real try/finally to | ||
| straight-line form — the leak is the missing Dispose, not the control flow. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the ShareX pattern this case is trying to model,
Stop()is the cleanup: in the .NET implementation used by the benchmark,TcpListener.Dispose()just delegates toStop(), andStop()disposes the underlying socket and clears it. Once this line runs (the real method does it infinally), there is no leaked listen socket, so adding this case with expectedOWN001and ratcheting CI to require it makes the corpus protect a false positive rather than a regression.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, and this is the important kind of catch — thank you.
TcpListener.Dispose()delegates toStop()(which disposes the listen socket), so aStop()'d listener holds no resource: flagging it OWN001 is a false positive, and locking it would have pinned an FP against our 0-FP principle. The same applies to the StreamReader case (itsMemoryStreamis separatelyusing'd).So I'm closing this PR rather than locking these. And since the extractor genuinely over-fires on
Stop()-cleanedTcpListener, that's a real precision gap worth fixing — I'll harden it to modeltcpListener.Stop()as a release (TcpListener-specific, resolved via the symbol;Stop()on a Timer/Process/etc. is untouched), so a never-stopped listener still leaks but a stopped one stays silent. Following up in a separate PR.Generated by Claude Code