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.

Sign up for GitHub

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

Jump to bottom

Turn nrefs into atomic_int #3370

Merged
merged 4 commits into from
Oct 15, 2024
Merged

Turn nrefs into atomic_int #3370

merged 4 commits into from
Oct 15, 2024

Commits on Oct 15, 2024

  1. Turn nrefs into atomic_int 

    Throughout the code base reference counting is done in a thread-unsave
way.

Turn nrefs members into atomic_int so they are thread-save
themselves.

Make sure to only use ++ and -- operators and avoid non atomic
combinations of reading and writing. As the nref can't change again
after reaching zero this is thread-save even as we look at the result
an instruction later. Drop all the static *Unlink functions.

Technically the *Free() functions should *Link() the instance after the
ref count has reached 0 and the decision to free it has been made.
Otherwise the cleanup code linking and freeing to will lead to a
recusions. This is omitted here as it is not needed.

This does not make the data structures thread save on its own. But it
gives a foundation on which further thread safety work can be based on.
    @ffesti @dmnks
    ffesti authored and dmnks committed Oct 15, 2024
    Configuration menu
    Copy the full SHA
    4e976b4 View commit details
    Browse the repository at this point in the history

  2. Turn rpmts->nrefs into atomic_int 

    Throughout the code base reference counting is done in a thread-unsave
way.

Turn nrefs into atomic_int so it is thread-save.

Make sure to only use ++ and -- operators and avoid non atomic
combinations of reading and writing.  As the nref can't change again
after reaching zero this is thread-save even as we look at the result
an instruction later. Drop all the static *Unlink functions.

Technically all the *Free() functions should *Link() the instance after the
ref count has reached 0 and the decision to free it has been made.
Otherwise the cleanup code linking and freeing to will lead to a
recusions. Right now only rpmts needs that. So we only do that here.

This does not make the data structures thread save on its own. But it
gives a good foundation.
    @ffesti @dmnks
    ffesti authored and dmnks committed Oct 15, 2024
    Configuration menu
    Copy the full SHA
    c1453ce View commit details
    Browse the repository at this point in the history

  3. Turn FD_t->nrefs into atomic_int 

    Throughout the code base reference counting is done in a thread-unsave
way.

Turn nrefs into atomic_int so they are thread-save themselves. As the
nref can't change again after reaching zero this is thread-save even
as we look at the result an instruction later.

FD_t is special in that fdFree return the instance after free iff it
still has not reached a ref count of 0. Unfortunately code in
rpmShowProgress() relies on that.

This does not make the data structures thread save on its own. But it
gives a foundation on which data locking can be implemented.
    @ffesti @dmnks
    ffesti authored and dmnks committed Oct 15, 2024
    Configuration menu
    Copy the full SHA
    4fc21ff View commit details
    Browse the repository at this point in the history

  4. rpmKeyring and rpmstrPool nrefs to atomic_int 

    These data structures use the content lock (aka mutex) to also protect the
reference counter. This is wrong and complicates freeing interconnected
data structures. This lock should only protect the content of the
instance.

Turn nrefs members into atomic_int so they are thread-save themselves.

Make sure to only use ++ and -- operators and avoid non atomic
combinations of reading and writing.  As the nref can't change again
after reaching zero this is thread-save even as we look at the result
an instruction later.

Make sure the data lock is not aquired for updating the nrefs members.

Technically the *Free() functions should *Link() the instance after the
ref count has reached 0 and the decision to free it has been made.
Otherwise the cleanup code linking and freeing to will lead to a
recusions. This is not neccessary here so it is omitted.
    @ffesti @dmnks
    ffesti authored and dmnks committed Oct 15, 2024
    Configuration menu
    Copy the full SHA
    87aad15 View commit details
    Browse the repository at this point in the history