Skip to content
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

Add public Network.close() method to perform controlled network shutdown #129

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

jagerman
Copy link
Member

@jagerman jagerman commented May 9, 2024

This renames the close_gracefully() method to just close() and makes
it public so that an application has a bit more control over Network
shutdown.

libsession-util ran into an issue where the quic::Network destruction
segfaults because it has callbacks (e.g. connection closing) that expect
to be able to use their local quic::Network instance, but can't
because it's in the process of being destroyed.

Essentially the pattern is this:

class A {
    quic::Network net;
};
A a;
// start a connection with a connection_close callback that
// references `a`.

If the quic::Network destruction is triggered by the destruction of a,
then the quic::Network destructor is firing after a is no longer in
a valid state, and so the callback referencing a is invalid.

Exposing close() allows the application to deal with this: it can add a
net.close() either in A's destructor or some other shutdown code so
that it can ensure the quic Network shutdown happens before a
becomes unusable.

This also adds a non-synchronous call_soon() to mirror the call(ep)
and call_soon(ep) methods for shutdown down individual endpoints.

@jagerman jagerman marked this pull request as draft May 27, 2024 18:41
This renames the close_gracefully() method to just `close()` and makes
it public so that an application has a bit more control over Network
shutdown.

libsession-util ran into an issue where the quic::Network destruction
segfaults because it has callbacks (e.g. connection closing) that expect
to be able to use their local `quic::Network` instance, but can't
because it's in the process of being destroyed.

Essentially the pattern is this:

    class A {
        quic::Network net;
    };
    A a;
    // start a connection with a connection_close callback that
    // references `a`.

If the quic::Network destruction is triggered by the destruction of `a`,
then the quic::Network destructor is firing *after* `a` is no longer in
a valid state, and so the callback referencing `a` is invalid.

Exposing close() allows the application to deal with this: it can add a
`net.close()` either in `A`'s destructor or some other shutdown code so
that it can ensure the quic Network shutdown happens *before* `a`
becomes unusable.

This also adds a non-synchronous `call_soon()` to mirror the call(ep)
and call_soon(ep) methods for shutdown down individual endpoints.
@jagerman jagerman force-pushed the expose-close-gracefully branch from d448470 to d013c28 Compare February 25, 2025 16:26
@jagerman jagerman marked this pull request as ready for review February 25, 2025 16:26
@jagerman jagerman changed the title Make close_gracefully public Add public Network.close() method to perform controlled network shutdown Feb 25, 2025
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.

1 participant