[python] Cleanup ThreadPool with atexit rather than __del__ (#5094)

* [python] Cleanup ThreadPool with atexit rather than __del__ This removes the `__del__` function from the generated Python client, and replaces it with a `cleanup` function. When a ThreadPool is created, the cleanup function is registered with the `atexit` module. This fixes #5093, where the API client could hang indefinitely at garbage collection. * Update petstore examples * Test to ensure threadpool is cleaned up * Docs now encourage using the context manager * Regenerate docs * Update samples
OpenAPITools · Jan 29, 2020 · 15345e1 · 15345e1
1 parent d627282
commit 15345e1
Show file tree

Hide file tree

Showing 58 changed files with 2,968 additions and 2,382 deletions.
diff --git a/modules/openapi-generator/src/main/resources/python/api_client.mustache b/modules/openapi-generator/src/main/resources/python/api_client.mustache
@@ -2,6 +2,7 @@
 {{>partial_header}}
 from __future__ import absolute_import
 
+import atexit
 import datetime
 from dateutil.parser import parse
 import json
@@ -75,18 +76,27 @@ class ApiClient(object):
         self.user_agent = '{{#httpUserAgent}}{{{.}}}{{/httpUserAgent}}{{^httpUserAgent}}OpenAPI-Generator/{{{packageVersion}}}/python{{/httpUserAgent}}'
         self.client_side_validation = configuration.client_side_validation
 
-    def __del__(self):
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        self.close()
+
+    def close(self):
         if self._pool:
             self._pool.close()
             self._pool.join()
             self._pool = None
+            if hasattr(atexit, 'unregister'):
+                atexit.unregister(self.close)
 
     @property
     def pool(self):
         """Create thread pool on first request
          avoids instantiating unused threadpool for blocking clients.
         """
         if self._pool is None:
+            atexit.register(self.close)
             self._pool = ThreadPool(self.pool_threads)
         return self._pool
 

diff --git a/modules/openapi-generator/src/main/resources/python/api_doc_example.mustache b/modules/openapi-generator/src/main/resources/python/api_doc_example.mustache
@@ -8,22 +8,26 @@ from pprint import pprint
 {{#hasAuthMethods}}
 # Defining host is optional and default to {{{basePath}}}
 configuration.host = "{{{basePath}}}"
-# Create an instance of the API class
-api_instance = {{{packageName}}}.{{{classname}}}({{{packageName}}}.ApiClient(configuration))
-{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
-{{/allParams}}
+# Enter a context with an instance of the API client
+with {{{packageName}}}.ApiClient(configuration) as api_client:
+    # Create an instance of the API class
+    api_instance = {{{packageName}}}.{{{classname}}}(api_client)
+    {{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
+    {{/allParams}}
 {{/hasAuthMethods}}
 {{^hasAuthMethods}}
-# Create an instance of the API class
-api_instance = {{{packageName}}}.{{{classname}}}()
-{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
-{{/allParams}}
+# Enter a context with an instance of the API client
+with {{{packageName}}}.ApiClient() as api_client:
+    # Create an instance of the API class
+    api_instance = {{{packageName}}}.{{{classname}}}(api_client)
+    {{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
+    {{/allParams}}
 {{/hasAuthMethods}}
 
-try:
-{{#summary}}    # {{{.}}}
-{{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
-    pprint(api_response){{/returnType}}
-except ApiException as e:
-    print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
+    try:
+    {{#summary}}    # {{{.}}}
+    {{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
+        pprint(api_response){{/returnType}}
+    except ApiException as e:
+        print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
 ```
diff --git a/modules/openapi-generator/src/main/resources/python/common_README.mustache b/modules/openapi-generator/src/main/resources/python/common_README.mustache
@@ -8,18 +8,20 @@ from pprint import pprint
 {{> python_doc_auth_partial}}
 # Defining host is optional and default to {{{basePath}}}
 configuration.host = "{{{basePath}}}"
-# Create an instance of the API class
-api_instance = {{{packageName}}}.{{{classname}}}({{{packageName}}}.ApiClient(configuration))
-{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
-{{/allParams}}
+# Enter a context with an instance of the API client
+with {{{packageName}}}.ApiClient(configuration) as api_client:
+    # Create an instance of the API class
+    api_instance = {{{packageName}}}.{{{classname}}}(api_client)
+    {{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
+    {{/allParams}}
 
-try:
-{{#summary}}    # {{{.}}}
-{{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
-    pprint(api_response){{/returnType}}
-except ApiException as e:
-    print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
-{{/-first}}{{/operation}}{{/operations}}{{/-first}}{{/apis}}{{/apiInfo}}
+    try:
+    {{#summary}}    # {{{.}}}
+    {{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
+        pprint(api_response){{/returnType}}
+    except ApiException as e:
+        print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
+    {{/-first}}{{/operation}}{{/operations}}{{/-first}}{{/apis}}{{/apiInfo}}
 ```
 
 ## Documentation for API Endpoints

diff --git a/...es/openapi-generator/src/main/resources/python/python-experimental/README_common.mustache b/...es/openapi-generator/src/main/resources/python/python-experimental/README_common.mustache
@@ -7,18 +7,20 @@ from pprint import pprint
 {{> python_doc_auth_partial}}
 # Defining host is optional and default to {{{basePath}}}
 configuration.host = "{{{basePath}}}"
-# Create an instance of the API class
-api_instance = {{{packageName}}}.{{{classname}}}({{{packageName}}}.ApiClient(configuration))
-{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
-{{/allParams}}
+# Enter a context with an instance of the API client
+with {{{packageName}}}.ApiClient(configuration) as api_client:
+    # Create an instance of the API class
+    api_instance = {{{packageName}}}.{{{classname}}}(api_client)
+    {{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
+    {{/allParams}}
 
-try:
-{{#summary}}    # {{{.}}}
-{{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
-    pprint(api_response){{/returnType}}
-except {{{packageName}}}.ApiException as e:
-    print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
-{{/-first}}{{/operation}}{{/operations}}{{/-first}}{{/apis}}{{/apiInfo}}
+    try:
+    {{#summary}}    # {{{.}}}
+    {{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
+        pprint(api_response){{/returnType}}
+    except {{{packageName}}}.ApiException as e:
+        print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
+    {{/-first}}{{/operation}}{{/operations}}{{/-first}}{{/apis}}{{/apiInfo}}
 ```
 
 ## Documentation for API Endpoints

diff --git a/modules/openapi-generator/src/main/resources/python/python-experimental/api_client.mustache b/modules/openapi-generator/src/main/resources/python/python-experimental/api_client.mustache
@@ -3,6 +3,7 @@
 from __future__ import absolute_import
 
 import json
+import atexit
 import mimetypes
 from multiprocessing.pool import ThreadPool
 import os
@@ -74,18 +75,27 @@ class ApiClient(object):
         # Set default User-Agent.
         self.user_agent = '{{#httpUserAgent}}{{{.}}}{{/httpUserAgent}}{{^httpUserAgent}}OpenAPI-Generator/{{{packageVersion}}}/python{{/httpUserAgent}}'
 
-    def __del__(self):
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        self.close()
+
+    def close(self):
         if self._pool:
             self._pool.close()
             self._pool.join()
             self._pool = None
+            if hasattr(atexit, 'unregister'):
+                atexit.unregister(self.close)
 
     @property
     def pool(self):
         """Create thread pool on first request
          avoids instantiating unused threadpool for blocking clients.
         """
         if self._pool is None:
+            atexit.register(self.close)
             self._pool = ThreadPool(self.pool_threads)
         return self._pool
 

diff --git a/.../openapi-generator/src/main/resources/python/python-experimental/api_doc_example.mustache b/.../openapi-generator/src/main/resources/python/python-experimental/api_doc_example.mustache
@@ -7,51 +7,55 @@ from pprint import pprint
 {{#hasAuthMethods}}
 # Defining host is optional and default to {{{basePath}}}
 configuration.host = "{{{basePath}}}"
-# Create an instance of the API class
-api_instance = {{{packageName}}}.{{{classname}}}({{{packageName}}}.ApiClient(configuration))
+# Enter a context with an instance of the API client
+with {{{packageName}}}.ApiClient(configuration) as api_client:
+    # Create an instance of the API class
+    api_instance = {{{packageName}}}.{{{classname}}}(api_client)
 {{/hasAuthMethods}}
 {{^hasAuthMethods}}
-# Create an instance of the API class
-api_instance = {{{packageName}}}.{{{classname}}}()
+# Enter a context with an instance of the API client
+with {{{packageName}}}.ApiClient(configuration) as api_client:
+    # Create an instance of the API class
+    api_instance = {{{packageName}}}.{{{classname}}}(api_client)
 {{/hasAuthMethods}}
-{{#requiredParams}}{{^defaultValue}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}
-{{/defaultValue}}{{/requiredParams}}{{#optionalParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} if omitted the server will use the default value of {{{defaultValue}}}{{/defaultValue}}
-{{/optionalParams}}
-{{#requiredParams}}
-{{^hasMore}}
+    {{#requiredParams}}{{^defaultValue}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}
+    {{/defaultValue}}{{/requiredParams}}{{#optionalParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} if omitted the server will use the default value of {{{defaultValue}}}{{/defaultValue}}
+    {{/optionalParams}}
+    {{#requiredParams}}
+    {{^hasMore}}
 
-# example passing only required values which don't have defaults set
-try:
-{{#summary}}    # {{{.}}}
-{{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#requiredParams}}{{^defaultValue}}{{paramName}}{{#hasMore}}, {{/hasMore}}{{/defaultValue}}{{/requiredParams}}){{#returnType}}
-    pprint(api_response){{/returnType}}
-except {{{packageName}}}.ApiException as e:
-    print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
-{{/hasMore}}
-{{/requiredParams}}
-{{#optionalParams}}
-{{^hasMore}}
+    # example passing only required values which don't have defaults set
+    try:
+    {{#summary}}    # {{{.}}}
+    {{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#requiredParams}}{{^defaultValue}}{{paramName}}{{#hasMore}}, {{/hasMore}}{{/defaultValue}}{{/requiredParams}}){{#returnType}}
+        pprint(api_response){{/returnType}}
+    except {{{packageName}}}.ApiException as e:
+        print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
+    {{/hasMore}}
+    {{/requiredParams}}
+    {{#optionalParams}}
+    {{^hasMore}}
 
-# example passing only required values which don't have defaults set
-# and optional values
-try:
-{{#summary}}    # {{{.}}}
-{{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#requiredParams}}{{^defaultValue}}{{paramName}}, {{/defaultValue}}{{/requiredParams}}{{#optionalParams}}{{paramName}}={{paramName}}{{#hasMore}}, {{/hasMore}}{{/optionalParams}}){{#returnType}}
-    pprint(api_response){{/returnType}}
-except {{{packageName}}}.ApiException as e:
-    print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
-{{/hasMore}}
-{{/optionalParams}}
-{{^requiredParams}}
-{{^optionalParams}}
+    # example passing only required values which don't have defaults set
+    # and optional values
+    try:
+    {{#summary}}    # {{{.}}}
+    {{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#requiredParams}}{{^defaultValue}}{{paramName}}, {{/defaultValue}}{{/requiredParams}}{{#optionalParams}}{{paramName}}={{paramName}}{{#hasMore}}, {{/hasMore}}{{/optionalParams}}){{#returnType}}
+        pprint(api_response){{/returnType}}
+    except {{{packageName}}}.ApiException as e:
+        print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
+    {{/hasMore}}
+    {{/optionalParams}}
+    {{^requiredParams}}
+    {{^optionalParams}}
 
-# example, this endpoint has no required or optional parameters
-try:
-{{#summary}}    # {{{.}}}
-{{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}(){{#returnType}}
-    pprint(api_response){{/returnType}}
-except {{{packageName}}}.ApiException as e:
-    print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
-{{/optionalParams}}
-{{/requiredParams}}
+    # example, this endpoint has no required or optional parameters
+    try:
+    {{#summary}}    # {{{.}}}
+    {{/summary}}    {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}(){{#returnType}}
+        pprint(api_response){{/returnType}}
+    except {{{packageName}}}.ApiException as e:
+        print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
+    {{/optionalParams}}
+    {{/requiredParams}}
 ```
diff --git a/samples/client/petstore/python-asyncio/README.md b/samples/client/petstore/python-asyncio/README.md
@@ -54,17 +54,19 @@ from pprint import pprint
 
 # Defining host is optional and default to http://petstore.swagger.io:80/v2
 configuration.host = "http://petstore.swagger.io:80/v2"
-# Create an instance of the API class
-api_instance = petstore_api.AnotherFakeApi(petstore_api.ApiClient(configuration))
-body = petstore_api.Client() # Client | client model
-
-try:
-    # To test special tags
-    api_response = api_instance.call_123_test_special_tags(body)
-    pprint(api_response)
-except ApiException as e:
-    print("Exception when calling AnotherFakeApi->call_123_test_special_tags: %s\n" % e)
-
+# Enter a context with an instance of the API client
+with petstore_api.ApiClient(configuration) as api_client:
+    # Create an instance of the API class
+    api_instance = petstore_api.AnotherFakeApi(api_client)
+    body = petstore_api.Client() # Client | client model
+
+    try:
+        # To test special tags
+        api_response = api_instance.call_123_test_special_tags(body)
+        pprint(api_response)
+    except ApiException as e:
+        print("Exception when calling AnotherFakeApi->call_123_test_special_tags: %s\n" % e)
+
 ```
 
 ## Documentation for API Endpoints

diff --git a/samples/client/petstore/python-asyncio/docs/AnotherFakeApi.md b/samples/client/petstore/python-asyncio/docs/AnotherFakeApi.md
@@ -23,16 +23,18 @@ import petstore_api
 from petstore_api.rest import ApiException
 from pprint import pprint
 
-# Create an instance of the API class
-api_instance = petstore_api.AnotherFakeApi()
-body = petstore_api.Client() # Client | client model
-
-try:
-    # To test special tags
-    api_response = api_instance.call_123_test_special_tags(body)
-    pprint(api_response)
-except ApiException as e:
-    print("Exception when calling AnotherFakeApi->call_123_test_special_tags: %s\n" % e)
+# Enter a context with an instance of the API client
+with petstore_api.ApiClient() as api_client:
+    # Create an instance of the API class
+    api_instance = petstore_api.AnotherFakeApi(api_client)
+    body = petstore_api.Client() # Client | client model
+
+    try:
+        # To test special tags
+        api_response = api_instance.call_123_test_special_tags(body)
+        pprint(api_response)
+    except ApiException as e:
+        print("Exception when calling AnotherFakeApi->call_123_test_special_tags: %s\n" % e)
 ```
 
 ### Parameters