Synchronous stopSendFeedback

- Android implementation returns a future which
  can be used by the caller to wait synchronously
  for the asynchronous stopSendFeedback call to
  complete
- iOS implementation is made to be synchronous
- Document further warnings

MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java

@@ -65,6 +65,7 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
 
 import psi.Psi;
 import psi.PsiphonProvider;
@@ -315,7 +316,10 @@ public class PsiphonTunnel {
     }
 
     // The interface for managing the Psiphon feedback upload operations.
-    // Warning: should not be used in the same process as PsiphonTunnel.
+    // Warnings:
+    // - Should not be used in the same process as PsiphonTunnel.
+    // - Only a single instance of PsiphonTunnelFeedback should be used at a time. Using multiple
+    // instances in parallel, or concurrently, will result in undefined behavior.
     public static class PsiphonTunnelFeedback {
 
         final private ExecutorService workQueue;
@@ -334,8 +338,13 @@ public class PsiphonTunnel {
         // HostFeedbackHandler. The provided HostLogger will be called to log informational notices,
         // including warnings.
         //
-        // Warning: only one active upload is supported at a time. An ongoing upload will be
-        // cancelled if this function is called again before it completes.
+        // Warnings:
+        // - Only one active upload is supported at a time. An ongoing upload will be cancelled if
+        // this function is called again before it completes.
+        // - An ongoing feedback upload started with startSendFeedback() should be stopped with
+        // stopSendFeedback() before the process exits. This ensures that any underlying resources
+        // are cleaned up; failing to do so may result in data store corruption or other undefined
+        // behavior.
         public void startSendFeedback(Context context, HostFeedbackHandler feedbackHandler, HostLogger logger,
                                       String feedbackConfigJson, String diagnosticsJson, String uploadPath,
                                       String clientPlatformPrefix, String clientPlatformSuffix) {
@@ -401,14 +410,16 @@ public class PsiphonTunnel {
             });
         }
 
-        // Interrupt an in-progress feedback upload operation started with startSendFeedback.
-        public void stopSendFeedback() {
-            workQueue.submit(new Runnable() {
+        // Interrupt an in-progress feedback upload operation started with startSendFeedback(). This
+        // call is asynchronous and returns a future which is fulfilled when the underlying stop
+        // operation completes.
+        public Future<Void> stopSendFeedback() {
+            return workQueue.submit(new Runnable() {
                 @Override
                 public void run() {
                     Psi.stopSendFeedback();
                 }
-            });
+            }, null);
         }
     }
 

MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.h

@@ -446,6 +446,8 @@ Returns the path where the rotated notices file will be created.
 /*!
  The interface for managing the Psiphon tunnel feedback upload operations.
  @warning Should not be used in the same process as PsiphonTunnel.
+ @warning Only a single instance of PsiphonTunnelFeedback should be used at a time. Using multiple instances in parallel, or
+ concurrently, will result in undefined behavior.
  */
 @interface PsiphonTunnelFeedback : NSObject
 
@@ -463,6 +465,9 @@ Returns the path where the rotated notices file will be created.
  error is non-nil, then the operation failed. Stored as a weak reference; the caller is responsible for holding a strong reference.
  @warning Only one active upload is supported at a time. An ongoing upload will be cancelled if this function is called again before it
  completes.
+ @warning An ongoing feedback upload started with `startSendFeedback:` should be stopped with `stopSendFeedback` before the
+ process exits. This ensures that any underlying resources are cleaned up; failing to do so may result in data store corruption or other
+ undefined behavior.
  */
 - (void)startSendFeedback:(NSString * _Nonnull)feedbackJson
        feedbackConfigJson:(id _Nonnull)feedbackConfigJson
@@ -470,7 +475,10 @@ Returns the path where the rotated notices file will be created.
            loggerDelegate:(id<PsiphonTunnelLoggerDelegate> _Nullable)loggerDelegate
          feedbackDelegate:(id<PsiphonTunnelFeedbackDelegate> _Nonnull)feedbackDelegate;
 
-/// Interrupt an in-progress feedback upload operation started with `startSendFeedback:`.
+/*!
+ Interrupt an in-progress feedback upload operation started with `startSendFeedback:`. This call is synchronous and returns once the
+ upload has been cancelled.
+ */
 - (void)stopSendFeedback;
 
 @end

MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m

@@ -1812,7 +1812,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
 
 // See comment in header.
 - (void)stopSendFeedback {
-    dispatch_async(self->workQueue, ^{
+    dispatch_sync(self->workQueue, ^{
         GoPsiStopSendFeedback();
     });
 }

MobileLibrary/psi/psi.go

@@ -317,6 +317,10 @@ var sendFeedbackWaitGroup *sync.WaitGroup
 //
 // Warnings:
 // - Should not be used with Start concurrently in the same process
+// - An ongoing feedback upload started with StartSendFeedback should be
+//   stopped with StopSendFeedback before the process exists. This ensures that
+//   any underlying resources are cleaned up; failing to do so may result in
+//   data store corruption or other undefined behavior.
 // - Start and StartSendFeedback both make an attempt to migrate persistent
 //   files from legacy locations in a one-time operation. If these functions
 //   are called in parallel, then there is a chance that the migration attempts