[GitHub] [activemq-artemis] wy96f commented on issue #2832: ARTEMIS-2482 Large messages could leak native ByteBuffers

2019-09-16 Thread GitBox
wy96f commented on issue #2832: ARTEMIS-2482 Large messages could leak native 
ByteBuffers
URL: https://github.com/apache/activemq-artemis/pull/2832#issuecomment-531688339
 
 
   As you said, size of tiny/small/medium caches is quite small. Even if not 
freed in time, the impact is very little compared to IOUtil that pool native 
ByteBuffers(default TEMP_BUF_POOL_SIZE=8) for each thread :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [activemq-artemis] wy96f commented on issue #2832: ARTEMIS-2482 Large messages could leak native ByteBuffers

2019-09-16 Thread GitBox
wy96f commented on issue #2832: ARTEMIS-2482 Large messages could leak native 
ByteBuffers
URL: https://github.com/apache/activemq-artemis/pull/2832#issuecomment-531654209
 
 
   @franz1981 
   
   > I have to add more detail and I was wrong: the tiny/small/medium thread 
local caches are actually leaking memory regions (with default 
io.netty.allocator.useCacheForAllThread) that need the holding thread to die in 
order to make them available to other threads again
   
   I assumed memory released into tiny/small/medium cache would be leaked if 
holding thread terminates until I looked at the code in PoolThreadCache:
   ```
   /// TODO: In the future when we move to Java9+ we should use 
java.lang.ref.Cleaner.
   @Override
   protected void finalize() throws Throwable {
   try {
   super.finalize();
   } finally {
   free();
   }
   }
   
   /**
*  Should be called if the Thread that uses this cache is about to 
exist to release resources out of the cache
*/
   void free() {
   // As free() may be called either by the finalizer or by 
FastThreadLocal.onRemoval(...) we need to ensure
   // we only call this one time.
   if (freed.compareAndSet(false, true)) {
   int numFreed = free(tinySubPageDirectCaches) +
   free(smallSubPageDirectCaches) +
   free(normalDirectCaches) +
   free(tinySubPageHeapCaches) +
   free(smallSubPageHeapCaches) +
   free(normalHeapCaches);
   
   if (numFreed > 0 && logger.isDebugEnabled()) {
   logger.debug("Freed {} thread-local buffer(s) from thread: 
{}", numFreed,
   Thread.currentThread().getName());
   }
   
   if (directArena != null) {
   directArena.numThreadCaches.getAndDecrement();
   }
   
   if (heapArena != null) {
   heapArena.numThreadCaches.getAndDecrement();
   }
   }
   }
   ```
   So the cache will be freed and used by other threads when thread dies? If 
so, it seems no any leak problems with this pr, no need with improvements?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [activemq-artemis] wy96f commented on issue #2832: ARTEMIS-2482 Large messages could leak native ByteBuffers

2019-09-11 Thread GitBox
wy96f commented on issue #2832: ARTEMIS-2482 Large messages could leak native 
ByteBuffers
URL: https://github.com/apache/activemq-artemis/pull/2832#issuecomment-530283213
 
 
   > @wy96f
   > 
   > > PooledByteBufAllocator::directBuffer uses java's ThreadLocal. Wouldn't 
this result in leak in the case if thread terminates due to idle timeout as in 
#2199 ?
   > 
   > Do you mean the thread local arena? Or the thread local NIO ByteBuffer?
   > Anyway, at the end of its usage we always release the `ByteBuf` and Netty 
will take care to release any referenced resources: it's a pool so it leaks "by 
definition"
   
   I mean tinySubPageDirectCaches/smallSubPageDirectCaches/normalDirectCaches 
in PoolThreadCache. PoolChunk would be added into these caches first if 
released(not yet into shared arena). Given PoolThreadCache is thread local, 
would theses caches leak if thread terminates?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services