[Bug] logCallbackHandle is null

[crimsonvspurple]

Library version used

1.15.1

Java version

17x86 temurin

Scenario

PublicClient (AcquireTokenInteractive, AcquireTokenByUsernamePassword)

Is this a new or an existing app?

The app is in production, I haven't upgraded MSAL, but started seeing this issue

Issue description and reproduction steps

Our MSAL integration haven't been used for a while after initial implementation and now we are testing it again and we get this weird issue that we haven't seen before.

Caused by: com.microsoft.aad.msal4j.MsalClientException: Error occurred when calling MSALRuntime logging API: Cannot invoke "com.microsoft.azure.javamsalruntime.LogCallbackHandle.release()" because "com.microsoft.azure.javamsalruntime.MsalRuntimeInterop.logCallbackHandle" is null
    at com.microsoft.aad.msal4jbrokers.Broker.enableBrokerLogging(Broker.java:228)

Do you have any idea what could cause this?

I don't understand this function. If it gets enableLogging=true, it checks if logCallbackHandle == null but if it gets false, it assumes logCallbackHandle is not null and attemps to call a function on it. Why?

And I can't even see this library code on github because it is behind a private repo.

Relevant code snippets

public static synchronized void enableLogging(boolean enableLogging) {
        if (enableLogging) {
            // Avoid calling logging APIs if logging is already enabled
            if (logCallbackHandle == null) {
                LogCallbackHandle handle = new LogCallbackHandle();

                ERROR_HELPER.checkMsalRuntimeError(MSALRUNTIME_LIBRARY.MSALRUNTIME_RegisterLogCallback(
                        logCallback, null, handle));

                // Assign the handle to the global variable only after a successful call to
                // RegisterLogCallback
                logCallbackHandle = handle;
            }

        } else {
            // According to comments in MSALRuntime's MSALRuntimeLogging.h, releasing the log
            // callback handle will de-register it
            logCallbackHandle.release();
            logCallbackHandle = null;
        }
    }

Expected behavior

No response

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Regression

No response

Solution and workarounds

No response

[crimsonvspurple]

        if (logCallbackHandle != null)
            logCallbackHandle.release();

during shutdown, null check is there.

This class defines a static variable logCallbackHandle and only sets it inside enableLogging function based on a boolean...

[Avery-Dunn]

Hello @crimsonvspurple : This is definitely a bug due to the potential null pointer exception, and we will add the null check you recommended in a future release (the actual code for that package is in the private repo you mentioned, and handled by a different team than regular MSAL Java)

However, for now a workaround would be to just not call that enableBrokerLogging() API unless you want to enable logging, since it seems like the null exception will only happen if you send it 'false'. By default it is not enabled anyway, and is mainly meant for debugging as it can be very verbose

[crimsonvspurple]

that is what i ended up doing.