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

nfd_gtk – perform all GTK+ operations in a forked process #15

Closed
wants to merge 6 commits into from

Conversation

datenwolf
Copy link

Hi, this pull request covers a small patch that augments the GTK+ code with a forking wrapper. This essentially encapsulates everything GTK+ into a separate process. Aside solving the GTK+ deinitialization issue this also improves robustness of the host program: If anything on the GTK side crashes the process (for example a bug in a thumbnail generator or similar), only the process dedicated to the file dialog goes down and the host process sees an error. Another benefit of forking the file dialog is, that in case a networked file system is accessed and the network blocks one can kill the file dialog process without tearing down the host.

Q&A:

  • When run in a memory debugger memory leaks are reported:
    Yes, the file dialog process exits without freeing the buffers it created. That would be like giving a house a cleanup and paint job just before the demolishion crew goes to work.
  • There's no proper serialization format being used in the pipe. It just transmits raw binary in-memory data.
    Yes, the pipe is accessed by literally the very same binary (with identical virtual addresses for everything) that runs in two separate processes and the pipe transfers simply connect identical memory layouts of objects on both processes. There's no need for a descriptive serialization format here. Admittedly, someone could inject nonsense into /proc/fd/${PIPE} but there are easier ways to mess up things.

@datenwolf datenwolf closed this Aug 21, 2016
@mlabbe
Copy link
Owner

mlabbe commented Aug 21, 2016

Hi Datenwolf, I am wondering why you opted to close this pull request?

@datenwolf
Copy link
Author

Because in the meantime I've leant that the way I implemented it can
lead to deadlocks in the subsequent child process, if the parent process
uses multithreading. I was unaware that glibc's implementation of malloc
uses a global lock. If the fork happens right at the moment when a
different thread is in a malloc critical section, the child process will
never see that malloc lock getting released and deadlock on the first
call of malloc.

There are also a number of other async unsafe functions in (g)libc,
glib, gobject and gtk so that the whole "fork a subprocess" method can
fail miserably.

Of course if the parent process doesn't use threads, it's a perfectly
safe method to go about this. Unfortunately multithreading opens up a
big can of worms and I didn't see those creeping into that particular
corner. Mea culpa.

Back to the drawing board.

Cheers,

dw

On 08/21/16 17:05, Michael Labbe wrote:

Hi Datenwolf, I am wondering why you opted to close this pull request?


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#15 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAR1BAvS5nsofbM9orc4adng_R6UGk-Qks5qiGlAgaJpZM4IuMks.

@mlabbe
Copy link
Owner

mlabbe commented Aug 22, 2016

Okay, got it. Thanks for letting me know!

For future reference, I am more likely to accept patches of this magnitude if they come in a side-by-side implementation. For instance, nfd_gtk_fork.c, which lets Linux users choose which way to go depending on their users' needs. It also lets existing users continue to use current functionality.

Thanks for giving it a shot!

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.

2 participants