openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG][csharp-netcore] Memory leak in ApiClient

Open dhedey opened this issue 4 years ago • 4 comments

Bug Report Checklist

  • [X] Have you provided a full/minimal spec to reproduce the issue?
  • [N/A] Have you validated the input using an OpenAPI validator (example)?
  • [X] Have you tested with the latest master to confirm the issue still exists?
  • [X] Have you searched for related issues/PRs?
  • [N/A] What's the actual output vs expected output?
  • [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

In csharp-netcore, in ApiClient, if the token source is long-lived, then this line is a memory leak due to the un-disposed LinkedTokenSource.

finalToken = CancellationTokenSource.CreateLinkedTokenSource(finalToken, tokenSource.Token).Token;

This has been causing memory leaks on production systems.

The finalToken isn't even used in the method - so this line can be removed, or the code fixed (to ensure the linked token source is disposed)

openapi-generator version

5.3.0

OpenAPI declaration file content or url

N/A

Generation Details

N/A

Steps to reproduce
  • Compile a client
  • Create a method which: ** Creates a CancellationTokenSource (var cts = new CancellationTokenSource(); var cancellationToken = cts.Token;) ** In a loop, runs / awaits 100 requests in series, passing in the cancellationToken ** Stick a break point at the end of the loop
  • Debug the program. At the break point, investigate cts._registrations.Callbacks - this is a linked list. If you follow the Next properties, observe that the linked list is 100 items long, from all the callbacks registered by the linked cancellation token sources.
Related issues/PRs

Can't see any...

Suggest a fix

In these files:

  • https://github.com/OpenAPITools/openapi-generator/blob/b05faefb9304839b6c0d1f1d5e56fcb82d2f2456/modules/openapi-generator/src/main/resources/csharp-netcore/libraries/httpclient/ApiClient.mustache
  • https://github.com/OpenAPITools/openapi-generator/blob/b05faefb9304839b6c0d1f1d5e56fcb82d2f2456/samples/client/petstore/csharp-netcore/OpenAPIClient-httpclient/src/Org.OpenAPITools/Client/ApiClient.cs

Either remove the whole if (configuration.Timeout > 0) block (as finalToken isn't used anyway!). Or fix it so that finalToken is used instead of cancellationToken later in the method, but be sure to wrap the dispose of the CancellationTokenSource - either via a using block, or via a finally statement with something like linkedCts?.Dispose().

dhedey avatar Jan 18 '22 00:01 dhedey

@dhedey thanks for reporting the issue and suggesting 2 possible fixes. Which one do you prefer? Do you mind filing a PR with a fix you prefer to start with?

wing328 avatar Jan 19 '22 16:01 wing328

Any news about this issue?

Havunen avatar Aug 30 '22 10:08 Havunen

@Havunen - it was fixed with the merge of this PR: https://github.com/OpenAPITools/openapi-generator/pull/13049 which is due to be released in 6.1.0: https://github.com/OpenAPITools/openapi-generator/milestone/43

dhedey avatar Aug 30 '22 10:08 dhedey

6.1.0 has been released so this can be now closed?

Havunen avatar Sep 21 '22 09:09 Havunen