jbossas / jboss-threads Goto Github PK

When I schedule a task and then cancel it the executor keeps references to all related objects until the scheduled time it passed.

Here you can see two tests, demonstrating the idea: https://github.com/Settis/jboss-threads/blob/memoryLeakReproducer/src/test/java/org/jboss/threads/ScheduledEnhancedQueueExecutorTest.java#L32

When I run it with a SingleThreadScheduledExecutor the object is collected after the task was cancelled. But EnhancedQueueExecutor keeps the link.

We are using jboss-cli-client.jar inside of Jolokia to extract metrics from several JBoss server instances using JMX.

We use Jolokia in proxy mode, it means that we connect to Jolokia using a HTTP/JSON API and Jolokia uses jmx-remoting to obtain the info from the JBoss servers.

After some days running and several thousands queries the Jolokia server runs out of heap memory (java.lang.OutOfMemoryError: GC overhead limit exceeded).

Looking at the different memory spaces, PS Old Gen reaches the max value and did not release.

Analyzing the heap dump with Eclipse MAT: attached image img-2019-03-27-203310.png

One instance of "java.lang.ThreadGroup" loaded by "<system class loader>" occupies 20.846.512 (41,08%) bytes. The instance is referenced by org.jboss.threads.JBossThreadFactory @ 0xe093f8e8 , loaded by "org.jboss.modules.ModuleClassLoader @ 0xe0814138". The memory is accumulated in one instance of "java.lang.ThreadGroup[]" loaded by "<system class loader>".

We were using jboss-threads 2.1.2.Final-redhat-1 (which comes with jboss-cli-client.jar in JBoss EAP 6). After seeing this fix we upgraded jboss-cli-client.jar to use version 2.3.2.Final-redhat-1 (we take that lib from the newer jboss-cli-client.jar of JBoss EAP 7 and put into the JBoss EAP 6 client, maybe that surgery is not really updating the lib used?)

I tried to compile jboss eap 6 from the sources but got a bunch of different errors.

I have also open an issue with Jolokia but as the problem comes from the jboss lib, looks like nothing can be done in that part.

Thanks!

This issue was imported from JIRA. The original issue is JBTHR-72.

I get this output on mvn clean install:

...
[INFO] --- buildnumber-maven-plugin:1.4:create (get-scm-revision) @ jboss-threads ---
[WARNING] Cannot get the revision information from the scm repository, proceeding with revision of UNKNOWN : 
No such provider: 's'.
[INFO] Storing buildNumber: UNKNOWN at timestamp: 1661778676385
[WARNING] Cannot get the branch information from the git repository: 
No such provider: 's'.
[WARNING] Cannot get the branch information from the scm repository, proceeding with UNKNOWN_BRANCH : 
No such provider: 's'.
[INFO] Storing buildScmBranch: UNKNOWN_BRANCH
[INFO] 
...

We probably don't need this at all. Maybe updating jboss-parent would fix it.

I was wondering why EnhancedQueueExecutorBase5 is licensed under the LGPL rather than Apache 2.

For some scenarios, reaching thread pool exhaustion can be a critical state always requiring thread dumps for an RCA. So it would be helpful for the executor to have a configurable option allowing an automatic dump upon thread exhaustion.

The tests for executor keep-alive are faulty. If they run on a slow system, then the jobs used will expire before or after they are expected to, causing invalid results.

Tests which depend on a sequence of concurrent events should use a strict enforcement of that sequencing of some sort. CountDownLatch is one choice but there are other options as well. Any test which depends on time-based behavior is faulty by design and will cause false negatives, which impacts development and release timelines.

Also, the diagnostic output mixes up "expected" and "actual" in some cases.

This issue was imported from JIRA. The original issue is JBTHR-67. It was originally reported by @dmlloyd.

EAP7-488 need coverage for the following aspects of EnhancedQueueExecutor:

• that keepalive setting is honored
• that core size setting is honored
• core prestart works for core threads
• core thread timeout works

This issue was imported from JIRA. The original issue is JBTHR-77. It was originally reported by @mnovak1.

Label beginner-friendly issues with a good-first-issue label, add a guide for new contributors, and a file identifying the code owners of this project.

This issue was imported from JIRA. The original issue is JBTHR-89. It was originally reported by @moulalis.

GitHub CI should test on Mac OS (on M1 if possible), as well as testing with 11, 17, and "latest" JDKs.

The pool should be usable by:

• XNIO
• EJB client async invocation
• Discovery async invocation

This issue was imported from JIRA. The original issue is JBTHR-54. It was originally reported by @dmlloyd.

According to the documentation of Future#cancel:

Attempts to cancel execution of this task. This method has no effect if the task is already completed or cancelled, or could not be cancelled for some other reason. Otherwise, if this task has not started when cancel is called, this task should never run. If the task has already started, then the mayInterruptIfRunning parameter determines whether the thread executing this task (when known by the implementation) is interrupted in an attempt to stop the task.

The return value from this method does not necessarily indicate whether the task is now cancelled; use isCancelled.

However, the ScheduledFuture class does not elaborate on this behavior when the task represents a repeating task. Instead, in ScheduledExecutorService, the doc for scheduleAtFixedRate says:

The sequence of task executions continues indefinitely until one of the following exceptional completions occur:

• The task is explicitly cancelled via the returned future.
• The executor terminates, also resulting in task cancellation.
• An execution of the task throws an exception. In this case calling get on the returned future will throw ExecutionException, holding the exception as its cause.

Subsequent executions are suppressed. Subsequent calls to isDone() on the returned future will return true.

The implementation in FutureTask does cancel the task if it is running but only if the mayInterruptIfRunning flag is given as true. In this case, if the caller is the first thread to request interruption of the task, then true is returned from cancel(true) for that caller only (subsequent attempts will return false). This is contrary to the Javadoc for that method, which says:

Returns: false if the task could not be cancelled, typically because it has already completed; true otherwise. If two or more threads cause a task to be cancelled, then at least one of them returns true. Implementations may provide stronger guarantees.

The implication here is that true is returned if the task could be cancelled. Not "was cancelled" or "might be cancelled" or "is cancellable", but "could be cancelled". A logical and semantic disaster.

Again referring to the JDK implementation, we can infer that the correct specification of the method must be closer to "Returns true if the task was successfully requested to cancel".

Bringing it back to planet Earth, the current scheduling implementation of EQE will return false if a scheduled task is requested to be cancelled while the task is running. Currently, setting the mayInterruptIfRunning flag will interrupt the task, but the task will still complete successfully.

The best possible way to modify the EQE behavior to match the JDK is probably to add a "pending cancel" state. In this state, the task is cancelled as soon as its execution completes. Transitioning a task to this state will cause cancel(xx) to return true. The isCancelled method would also return true when in this state.

I can see unexpected behavior in EnhancedQueueExecutor [1]. Even when I set allowsCoreThreadTimeOut to false, sometimes happens that pool size goes below core size by a few threads.

Test scenario:

• create EnhancedQueueExecutor with coreSize=3, maxSize=6, allowsCoreThreadTimeOut=false, keepAliveTimeout=1000ms
• submit 6 threads into executor and wait until all are running so maxSize is reached
• allow all threads to finish and wait some time for threads to timeout

Expected result is that poolSize will be 3 as coreSize=3 but sometimes happens that poolSize is 2 or 1.

This might not be a bug as EnhancedQueueExecutor estimates its pool size and might allow to timeout more threads than coreSize. I'm creating this jira so it's explained for future reference.

[1] https://github.com/jbossas/jboss-threads/blob/2.3.3.Final/src/main/java/org/jboss/threads/EnhancedQueueExecutor.java

[2] https://issues.jboss.org/browse/EAP7-488

This issue was imported from JIRA. The original issue is JBTHR-78. It was originally reported by @mnovak1.

Following our recent discussion, we should try to be less verbose when starting up our components.

Typically, we should avoid displaying the version:

[o.j.threads] (main) JBoss Threads version 3.0.0.Alpha4

This issue was imported from JIRA. The original issue is JBTHR-70. It was originally reported by @gsmet.

If the OS runs out of native threads, the JDK's response is to throw an OutOfMemoryError from Thread#start. Presently, EnhancedQueueExecutor catches such errors and wraps them into a RejectedExecutionException (as a suppressed throwable).

We can take other actions as well, including (but not limited to):

• Propagating the error directly instead of wrapping it (I don't recommend this though)
• Passing the error to the default thread uncaught exception handler, in the hopes that it has some sensible policy (for example, maybe this would trigger "crash on OOME" on some JVMs; this would have to be tested though because by default it is just null) (Update I just verified that there is no uncaught exception handler installed in this case, at least on Temurin)
• Logging a warning
• Force a System.abort() when caught (perhaps after logging a message, hopefully the log is autoflushing)