From: Derek Zhou via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Robert Pluim <rpluim@gmail.com>
Cc: Glenn Morris <rgm@gnu.org>,
40665@debbugs.gnu.org, Paul Eggert <eggert@cs.ucla.edu>,
Eli Zaretskii <eliz@gnu.org>
Subject: bug#40665: 28.0.50; tls hang on local ssl
Date: Fri, 31 Jul 2020 23:22:02 +0000 (UTC) [thread overview]
Message-ID: <86k0yjw9rb.fsf@mail.3qin.us> (raw)
In-Reply-To: <86y2qnaqnv.fsf@mail.3qin.us>
Robert,
My patch still apply cleanly to either emacs-27 or master branch. I don't know who
else can review this, so I am adding a few random people from the git
log, sorry for the spam. Just don't it to sit in the dust and got forgotten.
Derek
Derek Zhou writes:
> Robert Pluim writes:
>
>> OK, that does make sense, and might even be more correct, but itʼs a
>> bigger change. You'll need more than just me to agree with it.
>>
> Patch reworked:
>
> * before the select, check every interesting gnutls stream for
> available data in the buffer
> * if some of them hit, and either there is no wait_proc or the
> wait_proc is one of the gnutls streams with new data, set the select
> timeout to 0
> * after the select, merge the gnutls buffer status into the select
> returns
>
> The patch is not much longer than before, still a net reduction of code
> lines. I've done some light testing and haven't found any problem.
>
> diff --git a/src/process.c b/src/process.c
> index 91d426103d..783ce098b3 100644
> --- a/src/process.c
> +++ b/src/process.c
> @@ -5497,6 +5497,10 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
> }
> else
> {
> +#ifdef HAVE_GNUTLS
> + int tls_nfds;
> + fd_set tls_available;
> +#endif
> /* Set the timeout for adaptive read buffering if any
> process has non-zero read_output_skip and non-zero
> read_output_delay, and we are not reading output for a
> @@ -5566,6 +5570,36 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
> }
> #endif
>
> +#ifdef HAVE_GNUTLS
> + /* GnuTLS buffers data internally. We need to check if some
> + data is available in the buffers manually before the select.
> + And if so, we need to skip the select which could block */
> + FD_ZERO (&tls_available);
> + tls_nfds = 0;
> + for (channel = 0; channel < FD_SETSIZE; ++channel)
> + if (! NILP (chan_process[channel]) && FD_ISSET(channel, &Available))
> + {
> + struct Lisp_Process *p =
> + XPROCESS (chan_process[channel]);
> + if (p && p->gnutls_p && p->gnutls_state
> + && ((emacs_gnutls_record_check_pending
> + (p->gnutls_state))
> + > 0))
> + {
> + tls_nfds++;
> + eassert (p->infd == channel);
> + FD_SET (p->infd, &tls_available);
> + }
> + }
> + /* if wait_proc is somebody else, we have to wait in select as usual.
> + Otherwisr, clobber the timeout */
> + if ((tls_nfds > 0) &&
> + (!wait_proc ||
> + (wait_proc->infd >= 0 &&
> + FD_ISSET(wait_proc->infd, &tls_available))))
> + timeout = make_timespec (0, 0);
> +#endif
> +
> /* Non-macOS HAVE_GLIB builds call thread_select in xgselect.c. */
> #if defined HAVE_GLIB && !defined HAVE_NS
> nfds = xg_select (max_desc + 1,
> @@ -5584,60 +5618,22 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
> #endif /* !HAVE_GLIB */
>
> #ifdef HAVE_GNUTLS
> - /* GnuTLS buffers data internally. In lowat mode it leaves
> - some data in the TCP buffers so that select works, but
> - with custom pull/push functions we need to check if some
> - data is available in the buffers manually. */
> - if (nfds == 0)
> + /* merge tls_available into Available */
> + if (tls_nfds > 0)
> + {
> + if (nfds == 0 || (nfds < 0 && errno == EINTR))
> {
> - fd_set tls_available;
> - int set = 0;
> -
> - FD_ZERO (&tls_available);
> - if (! wait_proc)
> - {
> - /* We're not waiting on a specific process, so loop
> - through all the channels and check for data.
> - This is a workaround needed for some versions of
> - the gnutls library -- 2.12.14 has been confirmed
> - to need it. */
> - for (channel = 0; channel < FD_SETSIZE; ++channel)
> - if (! NILP (chan_process[channel]))
> - {
> - struct Lisp_Process *p =
> - XPROCESS (chan_process[channel]);
> - if (p && p->gnutls_p && p->gnutls_state
> - && ((emacs_gnutls_record_check_pending
> - (p->gnutls_state))
> - > 0))
> - {
> - nfds++;
> - eassert (p->infd == channel);
> - FD_SET (p->infd, &tls_available);
> - set++;
> - }
> - }
> - }
> - else
> - {
> - /* Check this specific channel. */
> - if (wait_proc->gnutls_p /* Check for valid process. */
> - && wait_proc->gnutls_state
> - /* Do we have pending data? */
> - && ((emacs_gnutls_record_check_pending
> - (wait_proc->gnutls_state))
> - > 0))
> - {
> - nfds = 1;
> - eassert (0 <= wait_proc->infd);
> - /* Set to Available. */
> - FD_SET (wait_proc->infd, &tls_available);
> - set++;
> - }
> - }
> - if (set)
> - Available = tls_available;
> + /* fast path, just copy */
> + nfds = tls_nfds;
> + Available = tls_available;
> }
> + else if (nfds > 0)
> + /* slow path, merge one by one.
> + Note: nfds does not need to be accurate, just positive is enough */
> + for (channel = 0; channel < FD_SETSIZE; ++channel)
> + if (FD_ISSET(channel, &tls_available))
> + FD_SET(channel, &Available);
> + }
> #endif
> }
>
next prev parent reply other threads:[~2020-07-31 23:22 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-16 14:06 bug#40665: 28.0.50; tls hang on local ssl Derek Zhou
2020-04-16 16:22 ` Robert Pluim
2020-04-16 18:22 ` Derek Zhou
2020-04-16 20:47 ` Derek Zhou
2020-04-16 23:01 ` Derek Zhou
2020-04-17 18:41 ` Derek Zhou
2020-04-18 2:44 ` Derek Zhou
2020-04-19 14:34 ` Robert Pluim
2020-04-19 15:34 ` Derek Zhou
2020-04-19 16:12 ` Robert Pluim
2020-04-21 0:27 ` Derek Zhou
2020-04-21 1:41 ` Derek Zhou
2020-04-21 7:39 ` Robert Pluim
2020-04-21 13:37 ` Derek Zhou
2020-04-21 14:03 ` Robert Pluim
2020-04-21 14:45 ` Derek Zhou
2020-04-21 15:02 ` Derek Zhou
2020-04-21 15:04 ` Robert Pluim
2020-04-21 20:20 ` Derek Zhou
2020-04-21 22:20 ` Derek Zhou
2020-04-21 22:29 ` Derek Zhou
2020-04-22 8:32 ` Robert Pluim
2020-04-22 12:25 ` Derek Zhou
2020-04-22 13:12 ` Robert Pluim
2020-04-22 15:16 ` Derek Zhou
2020-04-22 16:14 ` Robert Pluim
2020-04-22 16:57 ` Derek Zhou
2020-04-23 2:20 ` Derek Zhou
2020-04-24 14:23 ` Derek Zhou via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-04-24 14:29 ` Robert Pluim
2020-05-07 19:08 ` Derek Zhou via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-07-31 23:22 ` Derek Zhou via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2020-08-02 5:34 ` Lars Ingebrigtsen
2020-08-03 5:58 ` Lars Ingebrigtsen
2020-08-03 9:44 ` Robert Pluim
2020-08-03 13:57 ` Derek Zhou via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-08-03 16:03 ` Robert Pluim
2020-08-04 8:42 ` Lars Ingebrigtsen
2020-08-04 19:28 ` Derek Zhou via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-08-04 19:45 ` Lars Ingebrigtsen
2020-08-03 13:36 ` Derek Zhou via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-04-17 8:35 ` Robert Pluim
2020-04-18 2:46 ` Derek Zhou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86k0yjw9rb.fsf@mail.3qin.us \
--to=bug-gnu-emacs@gnu.org \
--cc=40665@debbugs.gnu.org \
--cc=derek@3qin.us \
--cc=eggert@cs.ucla.edu \
--cc=eliz@gnu.org \
--cc=rgm@gnu.org \
--cc=rpluim@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.