[jira] [Resolved] (WICKET-6822) AsynchronousPageStore Potential Memory Leak

["Martin Tzvetanov Grigorov (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/WICKET-6822?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Martin Tzvetanov Grigorov resolved WICKET-6822.
-----------------------------------------------
    Fix Version/s: 8.10.0
                   9.1.0
       Resolution: Fixed

> AsynchronousPageStore Potential Memory Leak
> -------------------------------------------
>
>                 Key: WICKET-6822
>                 URL: https://issues.apache.org/jira/browse/WICKET-6822
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 9.0.0, 8.9.0
>            Reporter: Martin Tzvetanov Grigorov
>            Assignee: Martin Tzvetanov Grigorov
>            Priority: Major
>             Fix For: 9.1.0, 8.10.0
>
>
> From the dev@ mailing list:
>  
> ------------------------------------------------
> In Wicket 8.3.0, I had to override AsynchronousPageStore to fix a memory
> leak. In looking at older versions of 9.0.0's version
> of AsynchronousPageStore , I thought the memory leak had been fixed, but
> now that 9.0.0 has been released (and maybe it was always that way and I
> wasn't seeing it right), it seems like the memory leak is still there.
> Here is 8.3.0's AsynchronousPageStore.PageSavingRunnable inner class with
> the memory leak:
>  private class PageSavingRunnable implements Runnable
>  {
>   @Override
>   public void run()
>   {
>    while (pageSavingThread != null)
>    {
>     Entry entry = null;
>     try
>     {
>      entry = entries.poll(POLL_WAIT, TimeUnit.MILLISECONDS);
>     }
>     catch (InterruptedException e)
>     {
>      log.debug("PageSavingRunnable:: Interrupted...");
>     }
>     if (entry != null && pageSavingThread != null)
>     {
>      log.debug("PageSavingRunnable:: Saving asynchronously: {}...", entry);
>      delegate.storePage(entry.sessionId, [entry.page|http://entry.page/]);
>      entryMap.remove(getKey(entry));
>     }
>    }
>   }
>  }
> Note that in the code above if an exception is thrown during the call:
> delegate.storePage(entry.sessionId, [entry.page|http://entry.page/])
> the call:
> entryMap.remove(getKey(entry))
> never happens, causing the entryMap to continue to gradually increase in
> size, eventually causing the heap to run out of memory.
> I switched those two lines to be:
>      entryMap.remove(getKey(entry));
>      delegate.storePage(entry.sessionId, [entry.page|http://entry.page/]);
> Now if an exception is thrown in delegate.storePage(), there is no longer a
> problem because the page entry has already been removed.  This has been
> confirmed in our code base where after making the change, we no longer had
> memory leak problems.
> Now in Wicket 9.0.0 very similar code looks like it will have the same
> problem. Note the following in AsynchronousPageStore.PageAddingRunnable:
>  private static class PageAddingRunnable implements Runnable
>  {
>   private static final Logger log =
> LoggerFactory.getLogger(PageAddingRunnable.class);
>   private final BlockingQueue<PendingAdd> queue;
>   private final ConcurrentMap<String, PendingAdd> map;
>   private final IPageStore delegate;
>   private PageAddingRunnable(IPageStore delegate, BlockingQueue<PendingAdd>
> queue,
>                              ConcurrentMap<String, PendingAdd> map)
>   {
>    this.delegate = delegate;
>    this.queue = queue;
>    this.map = map;
>   }
>   @Override
>   public void run()
>   {
>    while (!Thread.interrupted())
>    {
>     PendingAdd add = null;
>     try
>     {
>      add = queue.poll(POLL_WAIT, TimeUnit.MILLISECONDS);
>     }
>     catch (InterruptedException e)
>     {
>      Thread.currentThread().interrupt();
>     }
>     if (add != null)
>     {
>      log.debug("Saving asynchronously: {}...", add);
>      add.asynchronous = true;
>      delegate.addPage(add, [add.page|http://add.page/]);
>      map.remove(add.getKey());
>     }
>    }
>   }
>  }
> If an exception is thrown during the call:
> delegate.addPage(add, [add.page|http://add.page/]);
> The call to:
> map.remove(add.getKey());
> never happens, which will presumably still cause a memory leak.
> We're in the process of migrating from Wicket 8.x to 9.0.0 and will be
> running 9.0.0 in a production environment soon.  If needed, we'll try to
> implement a similar fix for this in 9.0.0,
> Do you think this is something that would be important enough to be looked
> into soon?
> Thanks for your time, and thanks for Wicket.  It's a great web framework.
> We've been using it for many years now.
> {color:#888888}--------------------------------
> Todd Heaps{color}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)