Skip to content

Memory leak in HandleBase #1063

Description

@Jakbejk

Library version used

com.microsoft.azure:msal4j-brokers:1.0.0

Java version

Eclipse Adoptium - 21.0.10+1

Scenario

PublicClient (AcquireTokenInteractive, AcquireTokenByUsernamePassword)

Is this a new or an existing app?

This is a new app or experiment

Issue description and reproduction steps

Hello, we have problem with high usage of memory when signing up with SSO using MSAL. I take Java Heap and I realized that there is around 64k instances of HandleBase#HandleFinalizerThread... But in Javadoc is written, there should be only one thread processing that "garbage":

// When the first Handle is created, start the finalizer thread that all Handles share.

Should not HANDLE_FINALIZER_THREAD in HandleBase be static to share handles across multiple instances and not to create new "infinite" loop for each operation?

Relevant code snippets

abstract class HandleBase extends LongByReference implements AutoCloseable {
    private static final Logger LOG = LoggerFactory.getLogger(HandleBase.class);

    protected LongByReference msalRuntimeHandle;
    ReleaseMethod releaseMethod;

    /**
     * Thread that manages cleaning up Handles and their PhantomReferences
     */
    private final HandleFinalizerThread HANDLE_FINALIZER_THREAD = new HandleFinalizerThread();

....
....
    /**
     * Thread which will start when the first Handle is created.
     * <p>
     * This thread will be responsible for releasing handles in scenarios where we can't release
     * them immediately, and as a fail-safe in case a handle isn't released properly
     */
    class HandleFinalizerThread extends Thread {
        private final Logger LOG = LoggerFactory.getLogger(HandleFinalizerThread.class);

        private ReferenceQueue<HandleBase> handleReferenceQueue = new ReferenceQueue<>();

        HandleFinalizerThread() {
            setDaemon(true);
        }

        /**
         * Create a new PhantomReference to a give Handle by creating a HandleFinalizer with this
         * Handle's value and release method <p> When the PhantomReference is the only remaining
         * reference to the Handle, the HandleFinalizer will appear in handleReferenceQueue and the
         * Handle will be released
         */
        void addReference(
                HandleBase handle, LongByReference msalRuntimeHandle, ReleaseMethod releaseMethod) {
            // When the first Handle is created, start the finalizer thread that all Handles share
            if (!HANDLE_FINALIZER_THREAD.isAlive()) {
                // Set up unknown exception handling for the thread, to ensure as much as possible
                // gets released cleanly
                HANDLE_FINALIZER_THREAD.setUncaughtExceptionHandler((th, ex) -> {
                    LOG.error(
                            "Unexpected exception in HandleFinalizerThread with {} open async handles. Will attempt to cancel any async operations before stopping thread.",
                            MsalRuntimeFuture.msalRuntimeFutures.size());

                    for (MsalRuntimeFuture future : MsalRuntimeFuture.msalRuntimeFutures.values()) {
                        future.cancelAsyncOperation();
                        future.handle.release();
                    }
                });

                HANDLE_FINALIZER_THREAD.start();
            }

            new HandleFinalizer(handle, msalRuntimeHandle, releaseMethod, handleReferenceQueue);
        }

        @Override
        public void run() {
            try {
                while (true) {
                    // Although this is an infinite loop, ReferenceQueue's remove() method causes it
                    // to wait until an entry appears in handleReferenceQueue. This will only happen
                    // when a Handle is reachable only through a PhantomReference, and can therefore
                    // be released
                    HandleFinalizer handleFinalizer = (HandleFinalizer)handleReferenceQueue.remove();
                    LOG.info("Found Handle with no references, closing.");
                    handleFinalizer.release();
                }
            } catch (InterruptedException e) {
                // Ideally, this will only run when the entire program shuts down, and most handles
                // will be released via their close() method if their in a try-with-resources block
                //
                // MsalRuntimeFuture.msalRuntimeFutures allows us to track async handles, so we can
                // at least guarantee they always get canceled/released

                LOG.error(
                        "HandleFinalizerThread interrupted with {} open async handles. Will attempt to cancel any async operations before stopping thread.",
                        MsalRuntimeFuture.msalRuntimeFutures.size());

                for (MsalRuntimeFuture future : MsalRuntimeFuture.msalRuntimeFutures.values()) {
                    future.cancelAsyncOperation();
                    future.handle.release();
                }
            }
        }
    }

Expected behavior

Created just on HandleFinalizerThread

Identity provider

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

Regression

No response

Solution and workarounds

Make HANDLE_FINALIZER_THREAD static

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs attentionAutomatically used when an issue is created through an issue templateuntriagedAutomatically used when an issue is created through an issue template

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions