Non-terminated threads in PowerOfTwoBufferPool and CommonExecutor prevent JEE application / OSGi bundle undeployment

XMLWordPrintableJSON

    • Type: Bug
    • Resolution: Unresolved
    • Priority: Unknown
    • None
    • Affects Version/s: None
    • Component/s: Internal
    • None
    • None
    • Java Drivers
    • None
    • None
    • None
    • None
    • None
    • None

      Both PowerOfTwoBufferPool and CommonExecutor (added in the backpressure project) instantiate Thread objects that run for as long as the JVM does.

      When a class C loaded by a class loader CL instantiates1 a thread, that thread, if started, prevents CL (and, consequently, all the classes it defined with their static state) from becoming phantom reachable, and, therefore, from becoming unreachable.

      We should fix both PowerOfTwoBufferPool and CommonExecutor:

      • The fix should be done in a single place and used by both.
      • Given that the CommonExecutor API is meant to become the single point of dealing with global (those that are not per-MongoClient) execution resources (threads and thread pools), we better move the pruner thread from PowerOfTwoBufferPool under the management of CommonExecutor (that may be either or a different object, which is accessible only via CommonExecutor, or maybe the existing CommonExecutor.singleThreadScheduler can be reused for running the pruning tasks).
      • We can do this not sooner than when the backpressure project is merged in main. We should not work on fixing PowerOfTwoBufferPool separately from CommonExecutor.

      There are two known conceptual ways of fixing the bug:

      1. Atomically count non-closed MongoClient instances, terminate global threads when the number of instances goes from 1 to 0, create global threads when the number of instances goes from 0 to 1. This approach is drafted in https://github.com/mongodb/mongo-java-driver/pull/2032, but we should not copy the draft, because it does not take into account CommonExecutor and how it affects the design as mentioned above.
        • Pros
          • Is easily applied to any situation because it does not depend on how a particular global thread is used.
          • The only additional contention it introduces is on the atomic counter, which is absolutely acceptable for us: we already have such global atomic counters, and our throughput/latency is not enough/low enough for a global atomic counter to have any effect.
        • Cons
          • Cannot be done merely by modifying PowerOfTwoBufferPool and CommonExecutor, requires a tiny involvement of MongoClientImpl. This so minor that is hardly important.
      2. Make each periodic task that runs by a global thread figure out whether another run is needed (for PowerOfTwoBufferPool that is if there are still buffers in the pool, for CommonExecutor at the moment that is if there are still scheduled tasks to run). If another run is not needed, then the periodic tasks terminates itself. When the condition changes (a buffer is added to the PowerOfTwoBufferPool, a task is scheduled via CommonExecutor), a new periodic task is scheduled. This approach is drafted in https://github.com/mongodb/mongo-java-driver/pull/2036, but we should not copy the draft, because it does not take into account CommonExecutor and how it affects the design as mentioned above.
        • Pros
          • Can be done merely by modifying PowerOfTwoBufferPool and CommonExecutor without involving anything else. This so minor that is hardly important.
        • Cons
          • A solution depends on how a specific global thread is used. It has to be crafted differently for different global threads.
          • For PowerOfTwoBufferPool, the thread may continue to run for the maxIdleTime duration (1 minute, currently) after all MongoClient instances having been closed, until running the task and terminating. That is because a pooled buffer becomes eligible for pruning only after maxIdleTime of having been unused. So the JEE server will still complain about the undeployed application classes and static state keeping the memory occupied, until the thread eventually terminates, allowing the application to be fully GCed.
          • For CommonExecutor, a grace period will have to be added to avoid terminating the thread in CommonExecutor.singleThreadScheduler when there are no scheduled tasks just because nothing happens to be scheduled at the moment, despite MongoClient instances existing and scheduling a task the next moment. Such a workaround then causes the same problem described above for PowerOfTwoBufferPool.

      At the moment, the first approach seems strictly better to me.

      When working on this ticket, we should also undo the change done in JAVA-6292.


      1 Surprisingly, it is indeed the instantiation of the Thread class that is important. Even if the runtime class of the Runnable run by the thread is defined by an ancestor class loader, and the Runnable instance doe not reference anything that could have held the child class loader, and the context class loader of the thread is an ancestor class loader, the Thread still prevents the child classloader from becoming unreachable for as long as it is not terminated. The code I used to conduct experiments is in https://github.com/stIncMale/mongo-java-driver/commit/862b7d75fa0629e2b7c9cc4d6e8761b1678934dd.

            Assignee:
            Unassigned
            Reporter:
            Valentin Kavalenka
            None
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: