From 8349b9610b114e0f65ac4921d4fa69fa5aa357ad Mon Sep 17 00:00:00 2001
From: "Bradley M. Kuhn" <bkuhn@ebb.org>
Date: Mon, 15 Jun 2020 14:42:33 -0700
Subject: [PATCH] Only server should control fifo; client should wait for it to
 clear
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On the a classic “concurrency is hard” front, I am slightly proud of
myself that I didn't hit this race condition but anticipated that it
might occur later.  Giving entire control to fifoName to the server
makes more sense here, and the client should not submit its query any
time either fifoName or question is set.

As an extra safeguard, the client will die if it gets the lock and
the state for submitting a query isn't right.

Finally, added some fifoName removal debugging on the server.
---
 bean-query-daemon-lib.pl    | 9 ++++++++-
 bean-query-goofy-daemon.plx | 8 +++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/bean-query-daemon-lib.pl b/bean-query-daemon-lib.pl
index 9745c9e..006d5ef 100644
--- a/bean-query-daemon-lib.pl
+++ b/bean-query-daemon-lib.pl
@@ -17,8 +17,15 @@ sub BeancountQueryInitialize {
 
 sub BeancountQuerySubmit($) {
   my($question) = @_;
+  while (defined $BEANCOUNT_QUERY{fifoName} or defined $BEANCOUNT_QUERY{question}) { sleep 1; }
   (tied %BEANCOUNT_QUERY)->shlock;
-  $BEANCOUNT_QUERY{fifoName} = undef;
+  if (defined $BEANCOUNT_QUERY{fifoName} or defined $BEANCOUNT_QUERY{question}) {
+    (tied %BEANCOUNT_QUERY)->shunlock;
+    no warnings 'uninitialized';
+    die("caught lock to submit a query, but either fifoName or question was defined, " .
+      "so something is wrong here.  " .
+      "fifoName: \"$BEANCOUNT_QUERY{fifoName}\" question: \"$BEANCOUNT_QUERY{question}\"!");
+  }
   $BEANCOUNT_QUERY{question} = $question;
   (tied %BEANCOUNT_QUERY)->shunlock;
   while (not defined $BEANCOUNT_QUERY{fifoName}) { sleep 1; }
diff --git a/bean-query-goofy-daemon.plx b/bean-query-goofy-daemon.plx
index 3ee8d93..8253451 100755
--- a/bean-query-goofy-daemon.plx
+++ b/bean-query-goofy-daemon.plx
@@ -71,11 +71,17 @@ tie %beancountData, 'IPC::Shareable', $glue, { %options } or
 
 while (1) {
   if (not defined $query{question}) {
-    print STDERR "No question posed, sleeping 2\n" if $VERBOSE > 2;
+    print STDERR "No question posed..." if $VERBOSE > 2;
     if (defined $query{fifoName}) {
+      print STDERR "fifo still active; locking to clear it..." if $VERBOSE > 2;
+      (tied %query)->shlock;
+      print STDERR "clearing fifo, $query{fifoName}..." if $VERBOSE > 2;
       no autodie 'unlink'; unlink($query{fifoName});
       $query{fifoName} = undef;
+      (tied %query)->shunlock;
+      print STDERR "fifo cleared & lock released." if $VERBOSE > 2;
     }
+    print STDERR "sleep for 2 seconds\n" if $VERBOSE > 2;
     sleep 2;
     next;
   } elsif ($query{question} !~  /^[\-\@\w.\s\"\'\_\(\)]+$/) {