Skip to content

Fix for missing disconnect when using gmp version autodetection#659

Closed
BaaaZen wants to merge 2 commits intogreenbone:mainfrom
BaaaZen:fix-missing-disconnect
Closed

Fix for missing disconnect when using gmp version autodetection#659
BaaaZen wants to merge 2 commits intogreenbone:mainfrom
BaaaZen:fix-missing-disconnect

Conversation

@BaaaZen
Copy link
Copy Markdown

@BaaaZen BaaaZen commented Jan 27, 2022

What:
We found out that only half of the SSH connections made were closed after leaving the Gmp context. After debugging the code we found out, that there is a problem when using the GMP version autodetection in combination with the use of the context manager.

Why:
This patch is a proposal to fix the problem with missing connection closes when using the GMP version autodetection.
The problem is caused by returning a new object (instead of self) from the context manager. Thus the connection is opened by the "new" object (the instance of the detected GMP protocol version) and closed by the detector object (Gmp class). So the connection was never closed, because the Gmp instance never opened it, and the "new" object never got a disconnect.

Maybe this is correlated with our support ticket GS-854?

How:
Instead of using the Gmp class directly for a context there is a new property supported_gmp that should be used. This circumvents the problem.

Checklist:
TBD

@BaaaZen BaaaZen requested a review from a team January 27, 2022 09:07
Comment thread gvm/protocols/gmp.py Outdated
Co-authored-by: Jaspar L. <jaspar.loechte@greenbone.net>
@y0urself
Copy link
Copy Markdown
Member

We think that this could be solved in a more beautiful way by just defining an __exit__() method that closes the connection, if still existing?

@BaaaZen
Copy link
Copy Markdown
Author

BaaaZen commented Jan 27, 2022

IMHO that won't work. When opening a context like this:

with Gmp(connection=connection) as gmp:
    print(gmp.get_version())

the __enter__ of the Gmp class creates a new object that opens the connection.
When leaving the context, the __exit__ of the Gmp class is called, not the __exit__ of the returned new instance (here in the gmp variable). So the disconnect() is not called for the connected new instance but for the not connected Gmp class.

The __exit__ method has no knowlege of the new instance that the __enter__ method returned. So the only way to solve this in our opinion is to move the generating of the new instance out of the context.

bjoernricks added a commit that referenced this pull request Jan 27, 2022
When using the context manager on the Gmp class which determines the
supported GMP version ensure that the opened connection is closed when
leaving the context.

Fixes #659
bjoernricks added a commit that referenced this pull request Jan 27, 2022
When using the context manager on the Gmp class which determines the
supported GMP version ensure that the opened connection is closed when
leaving the context.

Fixes #659
bjoernricks added a commit that referenced this pull request Jan 27, 2022
When using the context manager on the Gmp class which determines the
supported GMP version ensure that the opened connection is closed when
leaving the context.

Fixes #659
@bjoernricks
Copy link
Copy Markdown
Contributor

PR #660 addresses the problem without changing the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants