Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ endif()

if(MATSDK_BUILD_JNI_WRAPPER)
list(APPEND SRCS
jni/JavaDataViewerProxy.cpp
jni/JniConvertors.cpp
jni/LogManager_jni.cpp
jni/Logger_jni.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.microsoft.applications.events.DebugEventType;
import com.microsoft.applications.events.DiagLevel;
import com.microsoft.applications.events.HttpClient;
import com.microsoft.applications.events.IDataViewer;
import com.microsoft.applications.events.ILogConfiguration;
import com.microsoft.applications.events.ILogManager;
import com.microsoft.applications.events.ILogger;
Expand All @@ -42,7 +43,10 @@
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.FutureTask;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -247,6 +251,119 @@ public void startDDVonLogManager() {
LogManager.flushAndTeardown();
}

@Test
public void registerDataViewer_whenCallbackThrows_continuesDispatchAndStopsAfterUnregister()
throws Exception {
System.loadLibrary("maesdk");
Context appContext = InstrumentationRegistry.getInstrumentation().getTargetContext();
if (s_client == null) {
s_client = new MockHttpClient(appContext);
}
OfflineRoom.connectContext(appContext);

final String token =
"0123456789abcdef9123456789abcdef-01234567-0123-0123-0123-0123456789ab-0124";
final String factoryName = "JavaDataViewer" + System.nanoTime();
ILogConfiguration custom = LogManager.logConfigurationFactory();
custom.set(LogConfigurationKey.CFG_STR_PRIMARY_TOKEN, token);
custom.set(LogConfigurationKey.CFG_STR_COLLECTOR_URL, "https://viewer.contoso.com/");
custom.set(LogConfigurationKey.CFG_STR_FACTORY_NAME, factoryName);
custom.set(LogConfigurationKey.CFG_STR_CACHE_FILE_PATH, factoryName);

ILogManager manager = LogManagerProvider.createLogManager(custom);
CountDownLatch receivedPacket = new CountDownLatch(1);
AtomicInteger receivedByteCount = new AtomicInteger();
AtomicInteger receivingViewerCalls = new AtomicInteger();
AtomicInteger throwingViewerCalls = new AtomicInteger();
IDataViewer throwingViewer =
new IDataViewer() {
@Override
public void receiveData(byte[] packetData) {
throwingViewerCalls.incrementAndGet();
throw new IllegalStateException("Expected callback failure");
}

@Override
public String getName() {
return "throwing-viewer";
}

@Override
public boolean isTransmissionEnabled() {
return true;
}

@Override
public String getCurrentEndpoint() {
return "";
}
};
IDataViewer receivingViewer =
new IDataViewer() {
@Override
public void receiveData(byte[] packetData) {
receivingViewerCalls.incrementAndGet();
receivedByteCount.set(packetData.length);
receivedPacket.countDown();
}

@Override
public String getName() {
return "receiving-viewer";
}

@Override
public boolean isTransmissionEnabled() {
return true;
}

@Override
public String getCurrentEndpoint() {
return "http://127.0.0.1";
}
};

try {
assertThat(manager.registerDataViewer(throwingViewer), is(true));
assertThat(manager.registerDataViewer(receivingViewer), is(true));
assertThat(manager.registerDataViewer(receivingViewer), is(false));

ILogger logger = manager.getLogger(token, "java-data-viewer-test", "");
logger.logEvent("javaDataViewerCallback");
manager.uploadNow();

assertThat(receivedPacket.await(5, TimeUnit.SECONDS), is(true));
assertThat(receivedByteCount.get(), greaterThan(0));

assertThat(manager.unregisterDataViewer("receiving-viewer"), is(true));
assertThat(manager.unregisterDataViewer("receiving-viewer"), is(false));
Comment thread
Copilot marked this conversation as resolved.

// Unregistering must actually stop callbacks, not merely drop the bookkeeping entry: a
// bridge that left the proxy in the native DataViewerCollection would still pass the
// assertions above. Drive a second dispatch and use the still-registered throwing viewer
// as the witness that one really occurred, then assert the unregistered viewer was not
// called again.
final int receivingCallsAtUnregister = receivingViewerCalls.get();
final int throwingCallsAtUnregister = throwingViewerCalls.get();

logger.logEvent("javaDataViewerCallbackAfterUnregister");
manager.uploadNow();

final long deadline = System.currentTimeMillis() + 10000;
while (throwingViewerCalls.get() <= throwingCallsAtUnregister
&& System.currentTimeMillis() < deadline) {
Thread.sleep(50);
}

assertThat(throwingViewerCalls.get(), greaterThan(throwingCallsAtUnregister));
assertThat(receivingViewerCalls.get(), is(receivingCallsAtUnregister));

assertThat(manager.unregisterDataViewer("throwing-viewer"), is(true));
} finally {
manager.close();
}
}

/*
Disabling this test since it requires private modules.

Expand Down
4 changes: 4 additions & 0 deletions lib/android_build/maesdk/consumer-rules.pro
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-keep interface com.microsoft.applications.events.IDataViewer { *; }
-keep class * implements com.microsoft.applications.events.IDataViewer {
public *;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//
// Copyright (c) Microsoft Corporation. All rights reserved.
// SPDX-License-Identifier: Apache-2.0
//
package com.microsoft.applications.events;

import androidx.annotation.Keep;

/**
* Receives copies of packets uploaded by the SDK.
*
* <p>Implementations must return a stable, unique name for the lifetime of the registration.
* Callbacks can occur on an SDK worker thread and should return promptly. Implementations must not
* reenter the SDK from within a callback: do not register or unregister viewers, and do not close
* the owning {@link ILogManager}, because closing unregisters every viewer while the callback is
* still in progress.
*/
@Keep
public interface IDataViewer {

/** Receives an encoded telemetry packet after it has been prepared for upload. */
void receiveData(byte[] packetData);

/** Returns the stable, unique name used to register this viewer. */
String getName();

/** Returns whether this viewer is currently accepting packet callbacks. */
boolean isTransmissionEnabled();

/** Returns the endpoint currently used by this viewer, or an empty string when disabled. */
String getCurrentEndpoint();
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,34 @@ public interface ILogManager extends AutoCloseable {

public String getCurrentEndpoint();

/**
* Registers a caller-provided data viewer with this LogManager.
*
* <p>This is an optional capability. The default implementation returns {@code false} so that
* existing implementations of this interface remain source compatible; implementations that
* support data viewers override it.
*
* @return {@code true} when the viewer was registered, {@code false} for invalid input, a
* duplicate viewer name, or when the implementation does not support data viewers
*/
default boolean registerDataViewer(IDataViewer dataViewer) {
return false;
}

/**
* Unregisters a caller-provided data viewer by its unique name.
*
* <p>This is an optional capability. The default implementation returns {@code false} so that
* existing implementations of this interface remain source compatible; implementations that
* support data viewers override it.
*
* @return {@code true} when the viewer was unregistered, {@code false} when it was not
* registered, or when the implementation does not support data viewers
*/
default boolean unregisterDataViewer(String viewerName) {
return false;
}

public LogSessionData getLogSessionData();

public void setLevelFilter(int defaultLevel, int[] allowedLevels);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,28 @@ public String getCurrentEndpoint() {
return nativeGetCurrentEndpoint(nativeLogManager);
}

protected native boolean nativeRegisterDataViewer(
long nativeLogManager, IDataViewer dataViewer);

@Override
public boolean registerDataViewer(IDataViewer dataViewer) {
if (dataViewer == null) {
return false;
}
return nativeRegisterDataViewer(nativeLogManager, dataViewer);
}

protected native boolean nativeUnregisterDataViewer(
long nativeLogManager, String viewerName);

@Override
public boolean unregisterDataViewer(String viewerName) {
if (viewerName == null || viewerName.isEmpty()) {
return false;
}
return nativeUnregisterDataViewer(nativeLogManager, viewerName);
}

protected static class LogSessionDataImpl implements LogSessionData {
@Keep
private long m_first_time;
Expand Down
9 changes: 8 additions & 1 deletion lib/api/DataViewerCollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@ namespace MAT_NS_BEGIN {
return;

LOCKGUARD(m_dataViewerMapLock);
for(const auto& viewer : m_dataViewerCollection)
// Dispatch over a snapshot rather than the member directly. m_dataViewerMapLock is
// recursive, so a viewer that reenters the SDK from ReceiveData - for example by
// closing the owning LogManager, which unregisters every viewer - would otherwise
// erase from the very vector being iterated here and invalidate the iterator.
// Holding shared_ptr copies additionally keeps each viewer alive for the duration of
// its own callback, even if that callback drops the last other reference to it.
const auto viewers = m_dataViewerCollection;
for(const auto& viewer : viewers)
{
// Task 3568800: Integrate ThreadPool to IDataViewerCollection
viewer->ReceiveData(packetData);
Expand Down
Loading
Loading