public inbox for speakup@linux-speakup.org
 help / color / mirror / Atom feed
From: Okash Khawaja <okash.khawaja@gmail.com>
To: speakup@linux-speakup.org
Cc: John Covici <covici@ccs.covici.com>,
	Samuel Thibault <samuel.thibault@ens-lyon.org>
Subject: speakup-r empty line lockup
Date: Sat, 24 Jun 2017 10:06:45 +0100	[thread overview]
Message-ID: <20170624090645.GA8813@sanghar> (raw)

Hi,

The lockup when running speakup-r at start of an empty line occurs
because simulated key press is generated (using speakup_fake_down_arrow)
from context of keyboard_notifier_call callback which is called in
interrupt context. The simulated keypress leads to
keybaord_notifier_call to be trigerred again from the same context
leading to the lockup. The exact cause could be priority inversion where
simulated keypress cannot be processed because there is a real keyboard
interrupt already being processed (the one from which simulated keypress
was triggered), hence causing a deadlock. Please share your thoughts on
this. Here is the call chain.

(speakup-r) --> keyboard_notifier_call --> speakup_key --> do_spkup -->
read_all_doc --> get_sentence_buf [which returns -1 because of empty
line] --> kbd_fakekey2(RA_DOWN_ARROW) --> speakup_fake_down_arrow

The following patch resolves this by not simulating the keypress inside
keyboard notifier callback but instead delegating it to cursor_timer. In
the above chain, when get_sentence_buf returns -1, this patch starts
timer and passes RA_DOWN_ARROW as argument. When timer handler runs and
sees RA_DOWN_ARROW, it will then call kbd_fakekey2(RA_DOWN_ARROW) which
will correctly simulate the keypress inside timer context. I've tested
this succesfully.

It's the first time I've worked on this side of the code so please
review carefully :)

Finally, I have updated the test repo on github so you can test from
there.

Thanks,
Okash

---
 drivers/staging/speakup/main.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1408,7 +1408,8 @@ static void read_all_doc(struct vc_data
 	cursor_track = read_all_mode;
 	spk_reset_index_count(0);
 	if (get_sentence_buf(vc, 0) == -1) {
-		kbd_fakekey2(vc, RA_DOWN_ARROW);
+		del_timer(&cursor_timer);
+		start_read_all_timer(vc, RA_DOWN_ARROW);
 	} else {
 		say_sentence_num(0, 0);
 		synth_insert_next_index(0);

             reply	other threads:[~ UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
 Okash Khawaja [this message]
 ` Samuel Thibault
 ` Chris Brannon
 ` John Covici
   ` Okash Khawaja
 ` John Covici
   ` Okash Khawaja
     ` Okash Khawaja
     ` John Covici
       ` Okash Khawaja
       ` Okash Khawaja
         ` Chris Brannon
           ` John Covici
             ` Okash Khawaja
               ` Okash Khawaja
                 ` John Covici
                 ` Samuel Thibault
                   ` Okash Khawaja
                     ` Kirk Reiser
                       ` Chime Hart
                       ` Janina Sajka
                         ` John Covici
                           ` Samuel Thibault
                             ` John Covici
                               ` Okash Khawaja
                                 ` John Covici
                                   ` Okash Khawaja

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=20170624090645.GA8813@sanghar \
    --to=okash.khawaja@gmail.com \
    --cc=covici@ccs.covici.com \
    --cc=samuel.thibault@ens-lyon.org \
    --cc=speakup@linux-speakup.org \
    /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.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).