libextractor

GNU libextractor
Log | Files | Refs | Submodules | README | LICENSE

commit 6b22dcad7dc1027fedccf3719202afdcd6bf315f
parent 80a44ad71a8c5ba6a2b9238c7760ec4cc0d70cb7
Author: Christian Grothoff <christian@grothoff.org>
Date:   Sun, 29 Jan 2012 16:28:19 +0000

document threading issue found today (see also discussion on #gnunet

Diffstat:
MChangeLog | 4++++
Mdoc/extractor.texi | 41+++++++++++++++++++++++------------------
Msrc/main/extractor.c | 6++++--
3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog @@ -1,3 +1,7 @@ +Sun Jan 29 17:27:08 CET 2012 + Documented recently discovered issues with pthreads and + out-of-process plugin executions in the manual. -CG + Tue Nov 29 12:55:40 CET 2011 Improved IPC code on W32 to use APIs correctly and make it work on NT 6.1. -LRN diff --git a/doc/extractor.texi b/doc/extractor.texi @@ -15,7 +15,7 @@ This manual is for GNU libextractor GNU libextractor is a GNU package. -Copyright @copyright{} 2007, 2010 Christian Grothoff +Copyright @copyright{} 2007, 2010, 2012 Christian Grothoff @quotation Permission is granted to copy, distribute and/or modify this document @@ -535,26 +535,31 @@ and finally the extraction API itself. @cindex thread-safety @tindex enum EXTRACTOR_Options -All of the functions for loading and unloading plugins, including -@verb{|EXTRACTOR_plugin_add_defaults|} and @verb{|EXTRACTOR_plugin_remove_all|}, -are thread-safe and reentrant. However, using the same plugin list -from multiple threads at the same time is not safe. Creating multiple -plugin lists and using them concurrently is supported as long as -the @code{EXTRACTOR_OPTION_IN_PROCESS} option is not used. +Using @gnule{} from a multi-threaded parent process requires some +care. The problem is that on most platforms @gnule{} starts +sub-processes for the actual extraction work. This is useful to +isolate the parent process from potential bugs; however, it can cause +problems if the parent process is multi-threaded. The issue is that +at the time of the fork, another thread of the application may hold a +lock (i.e. in gettext or libc). That lock would then never be +released in the child process (as the other thread is not present in +the child process). As a result, the child process would then +deadlock on trying to acquire the lock and never terminate. This has +actually been observed with a lock in GNU gettext that is triggered by +the plugin startup code when it interacts with libltdl. + +The problem can be solved by loading the plugins using the +@code{EXTRACTOR_OPTION_IN_PROCESS} option, which will run @gnule{} +in-process and thus avoid the locking issue. In this case, all of the +functions for loading and unloading plugins, including +@verb{|EXTRACTOR_plugin_add_defaults|} and +@verb{|EXTRACTOR_plugin_remove_all|}, are thread-safe and reentrant. +However, using the same plugin list from multiple threads at the same +time is not safe. -Generally, @gnule{} is fully thread-safe and mostly reentrant. All plugin code is expected required to be reentrant and state-less, but due to the extensive use of 3rd party libraries this cannot -be guaranteed. Hence plugins are executed (by default) out of -process. This also ensures that plugins that crash do not cause -the main application to fail as well. - -Plugins can be executed in-process by giving the option -@code{EXTRACTOR_OPTION_IN_PROCESS} when loading the plugin. This -option is only recommended when debugging plugins and not for -production use. Due to the use of shared-memory IPC the -out-of-process execution of plugins should not be a concern for -performance. +be guaranteed. @deftp {C Struct} EXTRACTOR_PluginList diff --git a/src/main/extractor.c b/src/main/extractor.c @@ -1096,8 +1096,7 @@ write_all (int fd, static int -read_all ( - int fd, +read_all (int fd, void *buf, size_t size) { @@ -1688,6 +1687,9 @@ extract_oop (struct EXTRACTOR_PluginList *plugin, fflush (plugin->cpipe_in); while (1) { + fprintf (stderr, "Reading header from PID %u (plugin %s)\n", + plugin->cpid, + plugin->short_libname); if (0 != read_all (plugin->cpipe_out, &hdr, sizeof(hdr)))