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

Jump to bottom

[Bug]: Possible file descriptor leaks #4928

Closed
4 tasks done
mcrha opened this issue Jun 2, 2022 · 4 comments · Fixed by ostreedev/ostree#2692
Closed
4 tasks done

[Bug]: Possible file descriptor leaks #4928

mcrha opened this issue Jun 2, 2022 · 4 comments · Fixed by ostreedev/ostree#2692
Labels
bug

Comments

@mcrha
Copy link
Contributor

mcrha commented Jun 2, 2022

Checklist

  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for a bug that matches the one I want to file, without success.
  • If this is an issue with a particular app, I have tried filing it in the appropriate issue tracker for the app (e.g. under https://github.com/flathub/) and determined that it is an issue with Flatpak itself.
  • This issue is not a report of a security vulnerability (see here if you need to report a security issue).

Flatpak version

1.12.7

What Linux distribution are you using?

Fedora Linux

Linux distribution version

36

What architecture are you using?

x86_64

How to reproduce

Moving this form a downstream bug report:
https://bugzilla.redhat.com/show_bug.cgi?id=2090088

Which is also mirrored under gnome-software itself:
https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1783

Since the update to Fedora 36, which has flatpak-libs-1.12.7-2.fc36, there are leaked file descriptors after refreshing updates in the GNOME Software, pointing into:

~/.local/share/flatpak/repo/.lock
~/.local/share/flatpak/repo/tmp/cache
~/.local/share/flatpak/repo
~/.local/share/flatpak/repo/objects
~/.local/share/flatpak/repo/tmp

which can mean a regression in the flatpak itself. The same steps with flatpak-libs-1.12.6-1.fc35 do not reproduce the problem.

Expected Behavior

Well, no file descriptor leaks.

Actual Behavior

File descriptor leaks, which can leak to "too many opened files" errors, or even aborts of the application.

Additional Information

No response
@mcrha mcrha added the bug label Jun 2, 2022
@BrainBlasted
Copy link

BrainBlasted commented Jun 22, 2022 •
edited

I'm currently running into this - can't run flatpak update:

Warning: Error pulling from repo: GPG: Unable to complete signature verification: GPGME: Too many open files
Error: Error pulling from repo: GPG: Unable to complete signature verification: GPGME: Too many open files
Error: Error pulling from repo: GPG: Unable to complete signature verification: GPGME: Too many open files
Warning: Error pulling from repo: GPG: Unable to complete signature verification: GPGME: Too many open files
Error: Error pulling from repo: GPG: Unable to complete signature verification: GPGME: Too many open files
Error: Error pulling from repo: GPG: Unable to complete signature verification: GPGME: Too many open files
Warning: Failed to get revokefs-fuse socket from system-helper: Failed to duplicate file descriptor for child process (Too many open files)
Error: Error pulling from repo: GPG: Unable to complete signature verification: GPGME: Too many open files
Updates complete.
error: There were one or more errors

Using Flatpak 1.12.7

@GeorgesStavracas
Copy link
Member

Using Valgrind with --track-fds=true I managed to find which code path creates the leaked files:

==4788== Open file descriptor 223: /var/lib/flatpak/repo
==4788==    at 0x5A9ED50: openat (openat64.c:39)
==4788==    by 0x1092CB11: glnx_opendirat (glnx-dirfd.c:71)
==4788==    by 0x108C4377: ostree_repo_open (ostree-repo.c:3619)
==4788==    by 0x108C408D: reload_core_config (ostree-repo.c:3383)
==4788==    by 0x108C408D: ostree_repo_reload_config (ostree-repo.c:3571)
==4788==    by 0x108C44E1: ostree_repo_open (ostree-repo.c:3692)
==4788==    by 0x107DACEA: flatpak_dir_create_child_repo (flatpak-dir.c:9499)
==4788==    by 0x107DAFD7: flatpak_dir_create_system_child_repo (flatpak-dir.c:9548)
==4788==    by 0x107F02CF: flatpak_dir_update_appstream (flatpak-dir.c:5181)
==4788==    by 0x107FEED9: flatpak_installation_update_appstream_full_sync (flatpak-installation.c:2640)
==4788==    by 0x10761E21: gs_flatpak_refresh_appstream_remote (gs-flatpak.c:1418)
==4788==    by 0x1076212F: gs_flatpak_refresh_appstream (gs-flatpak.c:1491)
==4788==    by 0x1076416E: gs_flatpak_refresh (gs-flatpak.c:2198)
==4788==    by 0x1076FFF0: refresh_metadata_thread_cb (gs-plugin-flatpak.c:443)
==4788==    by 0x48C59F6: work_run_cb (gs-worker-thread.c:236)
==4788==    by 0x4C2EC6A: UnknownInlinedFun (gmain.c:3417)
==4788==    by 0x4C2EC6A: g_main_context_dispatch (gmain.c:4135)
==4788==    by 0x4C85000: g_main_context_iterate.constprop.0 (gmain.c:4211)
==4788==    by 0x4C2C391: g_main_context_iteration (gmain.c:4276)
==4788==    by 0x48C5838: thread_cb (gs-worker-thread.c:175)
==4788==    by 0x4C5E404: g_thread_proxy (gthread.c:827)
==4788==    by 0x5A2E78C: start_thread (pthread_create.c:442)
==4788==    by 0x5AAF8E3: clone (clone.S:100)
==4788== 
==4788== Open file descriptor 221: tmp
==4788==    at 0x5A9ED50: openat (openat64.c:39)
==4788==    by 0x1092CB11: glnx_opendirat (glnx-dirfd.c:71)
==4788==    by 0x108C44AF: ostree_repo_open (ostree-repo.c:3659)
==4788==    by 0x107DACEA: flatpak_dir_create_child_repo (flatpak-dir.c:9499)
==4788==    by 0x107DAFD7: flatpak_dir_create_system_child_repo (flatpak-dir.c:9548)
==4788==    by 0x107F02CF: flatpak_dir_update_appstream (flatpak-dir.c:5181)
==4788==    by 0x107FEED9: flatpak_installation_update_appstream_full_sync (flatpak-installation.c:2640)
==4788==    by 0x10761E21: gs_flatpak_refresh_appstream_remote (gs-flatpak.c:1418)
==4788==    by 0x1076212F: gs_flatpak_refresh_appstream (gs-flatpak.c:1491)
==4788==    by 0x1076416E: gs_flatpak_refresh (gs-flatpak.c:2198)
==4788==    by 0x1076FFF0: refresh_metadata_thread_cb (gs-plugin-flatpak.c:443)
==4788==    by 0x48C59F6: work_run_cb (gs-worker-thread.c:236)
==4788==    by 0x4C2EC6A: UnknownInlinedFun (gmain.c:3417)
==4788==    by 0x4C2EC6A: g_main_context_dispatch (gmain.c:4135)
==4788==    by 0x4C85000: g_main_context_iterate.constprop.0 (gmain.c:4211)
==4788==    by 0x4C2C391: g_main_context_iteration (gmain.c:4276)
==4788==    by 0x48C5838: thread_cb (gs-worker-thread.c:175)
==4788==    by 0x4C5E404: g_thread_proxy (gthread.c:827)
==4788==    by 0x5A2E78C: start_thread (pthread_create.c:442)
==4788==    by 0x5AAF8E3: clone (clone.S:100)
==4788== 
==4788== Open file descriptor 220: objects
==4788==    at 0x5A9ED50: openat (openat64.c:39)
==4788==    by 0x1092CB11: glnx_opendirat (glnx-dirfd.c:71)
==4788==    by 0x108C43E1: ostree_repo_open (ostree-repo.c:3629)
==4788==    by 0x107DACEA: flatpak_dir_create_child_repo (flatpak-dir.c:9499)
==4788==    by 0x107DAFD7: flatpak_dir_create_system_child_repo (flatpak-dir.c:9548)
==4788==    by 0x107F02CF: flatpak_dir_update_appstream (flatpak-dir.c:5181)
==4788==    by 0x107FEED9: flatpak_installation_update_appstream_full_sync (flatpak-installation.c:2640)
==4788==    by 0x10761E21: gs_flatpak_refresh_appstream_remote (gs-flatpak.c:1418)
==4788==    by 0x1076212F: gs_flatpak_refresh_appstream (gs-flatpak.c:1491)
==4788==    by 0x1076416E: gs_flatpak_refresh (gs-flatpak.c:2198)
==4788==    by 0x1076FFF0: refresh_metadata_thread_cb (gs-plugin-flatpak.c:443)
==4788==    by 0x48C59F6: work_run_cb (gs-worker-thread.c:236)
==4788==    by 0x4C2EC6A: UnknownInlinedFun (gmain.c:3417)
==4788==    by 0x4C2EC6A: g_main_context_dispatch (gmain.c:4135)
==4788==    by 0x4C85000: g_main_context_iterate.constprop.0 (gmain.c:4211)
==4788==    by 0x4C2C391: g_main_context_iteration (gmain.c:4276)
==4788==    by 0x48C5838: thread_cb (gs-worker-thread.c:175)
==4788==    by 0x4C5E404: g_thread_proxy (gthread.c:827)
==4788==    by 0x5A2E78C: start_thread (pthread_create.c:442)
==4788==    by 0x5AAF8E3: clone (clone.S:100)

@GeorgesStavracas
Copy link
Member

As far as I can see, flatpak_dir_pull() is leaking a ref to OstreeRepo

GeorgesStavracas added a commit to GeorgesStavracas/ostree that referenced this issue Aug 16, 2022
@GeorgesStavracas
lib/commit: Unref repo on success
092421f 
Commit 540e60c introduced _ostree_repo_auto_transaction_new(), a
private constructor to OstreeRepoAutoTransaction, by factoring out
some code from _ostree_repo_auto_transaction_start(). This factored
code increased the refcount of the 'repo' variable.

Subsequent commit 71304e8 made ostree_repo_prepare_transaction()
use ths newly introduced constructor. However, in this function, the
happy path assumed no ref was taken, and therefore did not unref it.
Commit 71304e8 didn't add the corresponding unref either.

This leaks a reference to OstreeRepo when calling
ostree_repo_prepare_transaction().

Plug this leak by using g_clear_object() to clear the repo field
of OstreeRepoAutoTransaction, instead of simply setting it to NULL.

Closes flatpak/flatpak#4928
@GeorgesStavracas GeorgesStavracas mentioned this issue Aug 16, 2022
Merged
@GeorgesStavracas
Copy link
Member

ostreedev/ostree#2692 should fix this.

@RDKRACZ RDKRACZ mentioned this issue Jan 6, 2023
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

lib/commit: Unref repo on success GeorgesStavracas/ostree
3 participants
@GeorgesStavracas @BrainBlasted @mcrha