diff options
| author | Pip Cet | 2020-08-21 14:56:06 +0200 |
|---|---|---|
| committer | Lars Ingebrigtsen | 2020-08-21 14:56:06 +0200 |
| commit | 9c62ffb08262c82b7e38e6eb5767f2087424aa47 (patch) | |
| tree | ea2a18eabffded063b40af0623b2485b10ad44dc /src | |
| parent | 19ee08f1e8599ce0e0465f6ffbd4a76791d791b4 (diff) | |
| download | emacs-9c62ffb08262c82b7e38e6eb5767f2087424aa47.tar.gz emacs-9c62ffb08262c82b7e38e6eb5767f2087424aa47.zip | |
Fix lock failures in xg_select
* src/xgselect.c (release_select_lock, acquire_select_lock):
Introduce.
(xg_select): Use `acquire_select_lock', `release_select_lock'.
* src/thread.c (release_select_lock): Introduce for non-GLib builds.
(really_call_select): Call `release_select_lock'. Simplify by
ensuring acquisition of the lock always succeeds (bug#36609).
Diffstat (limited to 'src')
| -rw-r--r-- | src/thread.c | 8 | ||||
| -rw-r--r-- | src/xgselect.c | 42 | ||||
| -rw-r--r-- | src/xgselect.h | 2 |
3 files changed, 38 insertions, 14 deletions
diff --git a/src/thread.c b/src/thread.c index b638dd77f8b..b4d8a53cf68 100644 --- a/src/thread.c +++ b/src/thread.c | |||
| @@ -28,6 +28,12 @@ along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */ | |||
| 28 | #include "pdumper.h" | 28 | #include "pdumper.h" |
| 29 | #include "keyboard.h" | 29 | #include "keyboard.h" |
| 30 | 30 | ||
| 31 | #ifdef HAVE_GLIB | ||
| 32 | #include <xgselect.h> | ||
| 33 | #else | ||
| 34 | #define release_select_lock() do { } while (0) | ||
| 35 | #endif | ||
| 36 | |||
| 31 | union aligned_thread_state | 37 | union aligned_thread_state |
| 32 | { | 38 | { |
| 33 | struct thread_state s; | 39 | struct thread_state s; |
| @@ -586,6 +592,8 @@ really_call_select (void *arg) | |||
| 586 | sa->result = (sa->func) (sa->max_fds, sa->rfds, sa->wfds, sa->efds, | 592 | sa->result = (sa->func) (sa->max_fds, sa->rfds, sa->wfds, sa->efds, |
| 587 | sa->timeout, sa->sigmask); | 593 | sa->timeout, sa->sigmask); |
| 588 | 594 | ||
| 595 | release_select_lock (); | ||
| 596 | |||
| 589 | block_interrupt_signal (&oldset); | 597 | block_interrupt_signal (&oldset); |
| 590 | /* If we were interrupted by C-g while inside sa->func above, the | 598 | /* If we were interrupted by C-g while inside sa->func above, the |
| 591 | signal handler could have called maybe_reacquire_global_lock, in | 599 | signal handler could have called maybe_reacquire_global_lock, in |
diff --git a/src/xgselect.c b/src/xgselect.c index f8d0bac7fac..be70107b756 100644 --- a/src/xgselect.c +++ b/src/xgselect.c | |||
| @@ -29,6 +29,27 @@ along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */ | |||
| 29 | #include "blockinput.h" | 29 | #include "blockinput.h" |
| 30 | #include "systime.h" | 30 | #include "systime.h" |
| 31 | 31 | ||
| 32 | static ptrdiff_t threads_holding_glib_lock; | ||
| 33 | static GMainContext *glib_main_context; | ||
| 34 | |||
| 35 | void release_select_lock (void) | ||
| 36 | { | ||
| 37 | if (--threads_holding_glib_lock == 0) | ||
| 38 | g_main_context_release (glib_main_context); | ||
| 39 | } | ||
| 40 | |||
| 41 | static void acquire_select_lock (GMainContext *context) | ||
| 42 | { | ||
| 43 | if (threads_holding_glib_lock++ == 0) | ||
| 44 | { | ||
| 45 | glib_main_context = context; | ||
| 46 | while (!g_main_context_acquire (context)) | ||
| 47 | { | ||
| 48 | /* Spin. */ | ||
| 49 | } | ||
| 50 | } | ||
| 51 | } | ||
| 52 | |||
| 32 | /* `xg_select' is a `pselect' replacement. Why do we need a separate function? | 53 | /* `xg_select' is a `pselect' replacement. Why do we need a separate function? |
| 33 | 1. Timeouts. Glib and Gtk rely on timer events. If we did pselect | 54 | 1. Timeouts. Glib and Gtk rely on timer events. If we did pselect |
| 34 | with a greater timeout then the one scheduled by Glib, we would | 55 | with a greater timeout then the one scheduled by Glib, we would |
| @@ -54,26 +75,19 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set *efds, | |||
| 54 | GPollFD *gfds = gfds_buf; | 75 | GPollFD *gfds = gfds_buf; |
| 55 | int gfds_size = ARRAYELTS (gfds_buf); | 76 | int gfds_size = ARRAYELTS (gfds_buf); |
| 56 | int n_gfds, retval = 0, our_fds = 0, max_fds = fds_lim - 1; | 77 | int n_gfds, retval = 0, our_fds = 0, max_fds = fds_lim - 1; |
| 57 | bool context_acquired = false; | ||
| 58 | int i, nfds, tmo_in_millisec, must_free = 0; | 78 | int i, nfds, tmo_in_millisec, must_free = 0; |
| 59 | bool need_to_dispatch; | 79 | bool need_to_dispatch; |
| 60 | 80 | ||
| 61 | context = g_main_context_default (); | 81 | context = g_main_context_default (); |
| 62 | context_acquired = g_main_context_acquire (context); | 82 | acquire_select_lock (context); |
| 63 | /* FIXME: If we couldn't acquire the context, we just silently proceed | ||
| 64 | because this function handles more than just glib file descriptors. | ||
| 65 | Note that, as implemented, this failure is completely silent: there is | ||
| 66 | no feedback to the caller. */ | ||
| 67 | 83 | ||
| 68 | if (rfds) all_rfds = *rfds; | 84 | if (rfds) all_rfds = *rfds; |
| 69 | else FD_ZERO (&all_rfds); | 85 | else FD_ZERO (&all_rfds); |
| 70 | if (wfds) all_wfds = *wfds; | 86 | if (wfds) all_wfds = *wfds; |
| 71 | else FD_ZERO (&all_wfds); | 87 | else FD_ZERO (&all_wfds); |
| 72 | 88 | ||
| 73 | n_gfds = (context_acquired | 89 | n_gfds = g_main_context_query (context, G_PRIORITY_LOW, &tmo_in_millisec, |
| 74 | ? g_main_context_query (context, G_PRIORITY_LOW, &tmo_in_millisec, | 90 | gfds, gfds_size); |
| 75 | gfds, gfds_size) | ||
| 76 | : -1); | ||
| 77 | 91 | ||
| 78 | if (gfds_size < n_gfds) | 92 | if (gfds_size < n_gfds) |
| 79 | { | 93 | { |
| @@ -151,8 +165,10 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set *efds, | |||
| 151 | #else | 165 | #else |
| 152 | need_to_dispatch = true; | 166 | need_to_dispatch = true; |
| 153 | #endif | 167 | #endif |
| 154 | if (need_to_dispatch && context_acquired) | 168 | if (need_to_dispatch) |
| 155 | { | 169 | { |
| 170 | acquire_select_lock (context); | ||
| 171 | |||
| 156 | int pselect_errno = errno; | 172 | int pselect_errno = errno; |
| 157 | /* Prevent g_main_dispatch recursion, that would occur without | 173 | /* Prevent g_main_dispatch recursion, that would occur without |
| 158 | block_input wrapper, because event handlers call | 174 | block_input wrapper, because event handlers call |
| @@ -162,11 +178,9 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set *efds, | |||
| 162 | g_main_context_dispatch (context); | 178 | g_main_context_dispatch (context); |
| 163 | unblock_input (); | 179 | unblock_input (); |
| 164 | errno = pselect_errno; | 180 | errno = pselect_errno; |
| 181 | release_select_lock (); | ||
| 165 | } | 182 | } |
| 166 | 183 | ||
| 167 | if (context_acquired) | ||
| 168 | g_main_context_release (context); | ||
| 169 | |||
| 170 | /* To not have to recalculate timeout, return like this. */ | 184 | /* To not have to recalculate timeout, return like this. */ |
| 171 | if ((our_fds > 0 || (nfds == 0 && tmop == &tmo)) && (retval == 0)) | 185 | if ((our_fds > 0 || (nfds == 0 && tmop == &tmo)) && (retval == 0)) |
| 172 | { | 186 | { |
diff --git a/src/xgselect.h b/src/xgselect.h index a38591f3296..512bf3ad85f 100644 --- a/src/xgselect.h +++ b/src/xgselect.h | |||
| @@ -29,4 +29,6 @@ extern int xg_select (int max_fds, | |||
| 29 | fd_set *rfds, fd_set *wfds, fd_set *efds, | 29 | fd_set *rfds, fd_set *wfds, fd_set *efds, |
| 30 | struct timespec *timeout, sigset_t *sigmask); | 30 | struct timespec *timeout, sigset_t *sigmask); |
| 31 | 31 | ||
| 32 | extern void release_select_lock (void); | ||
| 33 | |||
| 32 | #endif /* XGSELECT_H */ | 34 | #endif /* XGSELECT_H */ |