Multisession blocking repair
authorBill Erickson <berick@esilibrary.com>
Fri, 15 Mar 2013 19:56:55 +0000 (15:56 -0400)
committerGalen Charlton <gmc@esilibrary.com>
Sun, 17 Mar 2013 04:19:27 +0000 (21:19 -0700)
The new socket blocking code for multisession failed to take into
account that socket activity outside of the main block could lead to a
deadlock situation.  For example:

* Check status of request A -> not complete
* Check status of request B -> whatever
* Request A may now be complete, since checking the status of any request
  affects all requests
* Return to blocking loop because we think there is pending data, but in
  fact all data has already been pulled from the socket.

The solution is to make a sweep and check for changes in requests
without touching the socket after all of the socket-fiddling is done.

Signed-off-by: Bill Erickson <berick@esilibrary.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
src/perl/lib/OpenSRF/MultiSession.pm

index e196027..504d441 100644 (file)
@@ -176,6 +176,7 @@ sub request {
        my $self = shift;
        my $hash_param;
 
+
        my $method = shift;
        if (ref $method) {
                $hash_param = $method;
@@ -224,10 +225,12 @@ sub session_wait {
                        # block on the xmpp socket until data arrives
                        $xmpp->process(-1);
                        $self->session_reap;
+                       $self->session_reap while $self->check_post_reap_changes;
                }
                return $count;
        } else {
                while(($count = $self->session_reap) == 0 && $self->running) {
+                       $self->session_reap while $self->check_post_reap_changes;
                        # block on the xmpp socket until data arrives
                        $xmpp->process(-1);
                }
@@ -235,6 +238,18 @@ sub session_wait {
        }
 }
 
+# returns true if any running requests are marked as complete,
+# but does so without touching the underlying socket (note 
+# req->{complete} vs req->complete).  We do this so that checking
+# the state of the request does not affect the state of the 
+# request. With this, we can see if session_reap caused any state 
+# changes that were not accounted for within session_reap.
+sub check_post_reap_changes {
+       my $self = shift;
+       return 1 if grep { $_->{req}->{complete} } @{$self->{running}};
+       return 0;
+}
+
 sub session_reap {
        my $self = shift;