Merge branch 'master' into staging-client

MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java

@@ -1084,7 +1084,7 @@ private void handlePsiphonNotice(String noticeJSON) {
                       enableUdpGwKeepalive();
                     }
                 }
-                // Also report the tunnel's egress region to the host service
+            } else if (noticeType.equals("ConnectedServerRegion")) {
                 mHostService.onConnectedServerRegion(
                         notice.getJSONObject("data").getString("serverRegion"));
             } else if (noticeType.equals("ApplicationParameters")) {

MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m

@@ -1174,7 +1174,7 @@ - (void)handlePsiphonNotice:(NSString * _Nonnull)noticeJSON {
             });
         }
     }
-    else if ([noticeType isEqualToString:@"ActiveTunnel"]) {
+    else if ([noticeType isEqualToString:@"ConnectedServerRegion"]) {
         id region = [notice valueForKeyPath:@"data.serverRegion"];
         if (![region isKindOfClass:[NSString class]]) {
             [self logMessage:[NSString stringWithFormat: @"ActiveTunnel notice missing data.serverRegion: %@", noticeJSON]];

psiphon/controller.go

@@ -1001,8 +1001,9 @@ loop:
 			NoticeActiveTunnel(
 				connectedTunnel.dialParams.ServerEntry.GetDiagnosticID(),
 				connectedTunnel.dialParams.TunnelProtocol,
-				connectedTunnel.dialParams.ServerEntry.SupportsSSHAPIRequests(),
-				connectedTunnel.dialParams.ServerEntry.Region)
+				connectedTunnel.dialParams.ServerEntry.SupportsSSHAPIRequests())
+
+			NoticeConnectedServerRegion(connectedTunnel.dialParams.ServerEntry.Region)
 
 			if isFirstTunnel {
 

psiphon/notice.go

@@ -198,11 +198,12 @@ func setNoticeFiles(
 }
 
 const (
-	noticeIsDiagnostic   = 1
-	noticeIsHomepage     = 2
-	noticeClearHomepages = 4
-	noticeSyncHomepages  = 8
-	noticeSkipRedaction  = 16
+	noticeIsDiagnostic    = 1
+	noticeIsHomepage      = 2
+	noticeClearHomepages  = 4
+	noticeSyncHomepages   = 8
+	noticeSkipRedaction   = 16
+	noticeIsNotDiagnostic = 32
 )
 
 // outputNotice encodes a notice in JSON and writes it to the output writer.
@@ -279,17 +280,22 @@ func (nl *noticeLogger) outputNotice(noticeType string, noticeFlags uint32, args
 	if nl.rotatingFile != nil {
 
 		if !skipWriter {
-			skipWriter = (noticeFlags&noticeIsDiagnostic != 0)
+			// Skip writing to the host application if the notice is diagnostic
+			// and not explicitly marked as not diagnostic
+			skipWriter = (noticeFlags&noticeIsDiagnostic != 0) && (noticeFlags&noticeIsNotDiagnostic == 0)
 		}
 
 		if !skipRedaction {
+			// Only write to the rotating file if the notice is not explicitly marked as not diagnostic.
+			if noticeFlags&noticeIsNotDiagnostic == 0 {
 
-			err := nl.outputNoticeToRotatingFile(output)
+				err := nl.outputNoticeToRotatingFile(output)
 
-			if err != nil {
-				output := makeNoticeInternalError(
-					fmt.Sprintf("write rotating file failed: %s", err))
-				nl.writer.Write(output)
+				if err != nil {
+					output := makeNoticeInternalError(
+						fmt.Sprintf("write rotating file failed: %s", err))
+					nl.writer.Write(output)
+				}
 			}
 		}
 	}
@@ -678,12 +684,18 @@ func NoticeRequestedTactics(dialParams *DialParameters) {
 }
 
 // NoticeActiveTunnel is a successful connection that is used as an active tunnel for port forwarding
-func NoticeActiveTunnel(diagnosticID, protocol string, isTCS bool, serverRegion string) {
+func NoticeActiveTunnel(diagnosticID, protocol string, isTCS bool) {
 	singletonNoticeLogger.outputNotice(
 		"ActiveTunnel", noticeIsDiagnostic,
 		"diagnosticID", diagnosticID,
 		"protocol", protocol,
-		"isTCS", isTCS,
+		"isTCS", isTCS)
+}
+
+// NoticeConnectedServerRegion reports the region of the connected server
+func NoticeConnectedServerRegion(serverRegion string) {
+	singletonNoticeLogger.outputNotice(
+		"ConnectedServerRegion", 0,
 		"serverRegion", serverRegion)
 }
 
@@ -832,10 +844,12 @@ func NoticeClientUpgradeDownloaded(filename string) {
 // transferred since the last NoticeBytesTransferred. This is not a diagnostic
 // notice: the user app has requested this notice with EmitBytesTransferred
 // for functionality such as traffic display; and this frequent notice is not
-// intended to be included with feedback.
+// intended to be included with feedback. The noticeIsNotDiagnostic flag
+// ensures that these notices are emitted to the host application but not written
+// to the diagnostic file to avoid cluttering it with frequent updates.
 func NoticeBytesTransferred(diagnosticID string, sent, received int64) {
 	singletonNoticeLogger.outputNotice(
-		"BytesTransferred", 0,
+		"BytesTransferred", noticeIsNotDiagnostic,
 		"diagnosticID", diagnosticID,
 		"sent", sent,
 		"received", received)

psiphon/server/discovery.go

@@ -110,8 +110,6 @@ func (d *Discovery) reload(reloadedTactics bool) error {
 	// Initialize and set underlying discovery component. Replaces old
 	// component if discovery is already initialized.
 
-	oldDiscovery := d.discovery
-
 	discovery := discovery.MakeDiscovery(
 		d.support.PsinetDatabase.GetDiscoveryServers(),
 		discoveryStrategy)
@@ -120,6 +118,7 @@ func (d *Discovery) reload(reloadedTactics bool) error {
 
 	d.Lock()
 
+	oldDiscovery := d.discovery
 	d.discovery = discovery
 	d.currentStrategy = strategy
 
@@ -143,6 +142,8 @@ func (d *Discovery) reload(reloadedTactics bool) error {
 
 // Stop stops discovery and cleans up underlying resources.
 func (d *Discovery) Stop() {
+	d.Lock()
+	defer d.Unlock()
 	d.discovery.Stop()
 }
 

psiphon/server/discovery/discovery.go

@@ -167,7 +167,8 @@ func (d *Discovery) Start() {
 			// Note: servers with a discovery date range in the past are not
 			// removed from d.all in case the wall clock has drifted;
 			// otherwise, we risk removing them prematurely.
-			servers, nextUpdate := discoverableServers(d.all, d.clk)
+			var servers []*psinet.DiscoveryServer
+			servers, nextUpdate = discoverableServers(d.all, d.clk)
 
 			// Update the set of discoverable servers.
 			d.strategy.serversChanged(servers)

psiphon/server/discovery/discovery_test.go

@@ -149,9 +149,9 @@ func runDiscoveryTest(tt *discoveryTest, now time.Time) error {
 	discovery.Start()
 
 	for _, check := range tt.checks {
-		time.Sleep(1 * time.Second) // let async code complete
+		time.Sleep(10 * time.Millisecond) // let async code complete
 		clk.SetNow(check.t)
-		time.Sleep(1 * time.Second) // let async code complete
+		time.Sleep(10 * time.Millisecond) // let async code complete
 		discovered := discovery.SelectServers(net.IP{})
 		discoveredIPs := make([]string, len(discovered))
 		for i := range discovered {