diff --git a/csharp/ql/src/API Abuse/NoDisposeCallOnLocalIDisposable.ql b/csharp/ql/src/API Abuse/NoDisposeCallOnLocalIDisposable.ql index 3072b154812f..e5826c423427 100644 --- a/csharp/ql/src/API Abuse/NoDisposeCallOnLocalIDisposable.ql +++ b/csharp/ql/src/API Abuse/NoDisposeCallOnLocalIDisposable.ql @@ -16,6 +16,7 @@ import csharp import Dispose import semmle.code.csharp.frameworks.System +import semmle.code.csharp.frameworks.system.threading.Tasks import semmle.code.csharp.commons.Disposal private class ReturnNode extends DataFlow::ExprNode { @@ -24,15 +25,27 @@ private class ReturnNode extends DataFlow::ExprNode { } } +private class Task extends Type { + Task() { + this instanceof SystemThreadingTasksTaskClass or + this instanceof SystemThreadingTasksTaskTClass + } +} + module DisposeCallOnLocalIDisposableConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node node) { - node.asExpr() = - any(LocalScopeDisposableCreation disposable | - // Only care about library types - user types often have spurious IDisposable declarations - disposable.getType().fromLibrary() and - // WebControls are usually disposed automatically - not disposable.getType() instanceof WebControl - ) + exists(LocalScopeDisposableCreation disposable, Type t | + node.asExpr() = disposable and + t = disposable.getType() + | + // Only care about library types - user types often have spurious IDisposable declarations + t.fromLibrary() and + // WebControls are usually disposed automatically + not t instanceof WebControl and + // It is typically not nessesary to dispose tasks + // https://devblogs.microsoft.com/pfxteam/do-i-need-to-dispose-of-tasks/ + not t instanceof Task + ) } predicate isSink(DataFlow::Node node) { diff --git a/csharp/ql/src/change-notes/2025-03-10-task-not-disposed.md b/csharp/ql/src/change-notes/2025-03-10-task-not-disposed.md new file mode 100644 index 000000000000..faf748d873f6 --- /dev/null +++ b/csharp/ql/src/change-notes/2025-03-10-task-not-disposed.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cs/local-not-disposed` query no longer flags un-disposed tasks as this is often not needed (explained [here](https://devblogs.microsoft.com/pfxteam/do-i-need-to-dispose-of-tasks/)). diff --git a/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs b/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs index ae9bccf0e6e2..457c1ae4b40d 100644 --- a/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs +++ b/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs @@ -4,6 +4,7 @@ using System.IO.Compression; using System.Xml; using System.Threading; +using System.Threading.Tasks; class Test { @@ -86,6 +87,13 @@ public IDisposable Method() using (XmlReader.Create(source ?? new StringReader("xml"), null)) ; + // GOOD: Flagging these generates too much noise and there is a general + // acceptance that Tasks are not disposed. + // https://devblogs.microsoft.com/pfxteam/do-i-need-to-dispose-of-tasks/ + Task t = new Task(() => { }); + t.Start(); + t.Wait(); + return null; } diff --git a/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.expected b/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.expected index 1d71aa4af027..1a72517edb8b 100644 --- a/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.expected +++ b/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.expected @@ -1,7 +1,7 @@ -| NoDisposeCallOnLocalIDisposable.cs:50:19:50:38 | object creation of type Timer | Disposable 'Timer' is created but not disposed. | -| NoDisposeCallOnLocalIDisposable.cs:51:18:51:73 | object creation of type FileStream | Disposable 'FileStream' is created but not disposed. | -| NoDisposeCallOnLocalIDisposable.cs:52:9:52:64 | object creation of type FileStream | Disposable 'FileStream' is created but not disposed. | -| NoDisposeCallOnLocalIDisposable.cs:74:25:74:71 | call to method Create | Disposable 'XmlReader' is created but not disposed. | -| NoDisposeCallOnLocalIDisposable.cs:74:42:74:64 | object creation of type StringReader | Disposable 'StringReader' is created but not disposed. | -| NoDisposeCallOnLocalIDisposable.cs:79:38:79:67 | object creation of type StreamWriter | Disposable 'StreamWriter' is created but not disposed. | +| NoDisposeCallOnLocalIDisposable.cs:51:19:51:38 | object creation of type Timer | Disposable 'Timer' is created but not disposed. | +| NoDisposeCallOnLocalIDisposable.cs:52:18:52:73 | object creation of type FileStream | Disposable 'FileStream' is created but not disposed. | +| NoDisposeCallOnLocalIDisposable.cs:53:9:53:64 | object creation of type FileStream | Disposable 'FileStream' is created but not disposed. | +| NoDisposeCallOnLocalIDisposable.cs:75:25:75:71 | call to method Create | Disposable 'XmlReader' is created but not disposed. | +| NoDisposeCallOnLocalIDisposable.cs:75:42:75:64 | object creation of type StringReader | Disposable 'StringReader' is created but not disposed. | +| NoDisposeCallOnLocalIDisposable.cs:80:38:80:67 | object creation of type StreamWriter | Disposable 'StreamWriter' is created but not disposed. | | NoDisposeCallOnLocalIDisposableBad.cs:8:22:8:56 | object creation of type FileStream | Disposable 'FileStream' is created but not disposed. |