Re: [Bug# 228886] Multiple Glassfish threads hanging in memory

I tried attached jars in NB7.3.1 and there exists only one FetchLogPiped 
thread within live threads at the time. I haven't noticed any regression 
so far.

Jiri

On 22.5.2013 14:53, Tomas Kraus wrote:
> OK, here are simple changes. I tested it using
> https://netbeans.org/bugzilla/show_bug.cgi?id=228886#c12 and looks like
> there are no old threads left.
>
> Jiri, please test it too to be sure. JARs were built using clean
> release73 branch.
>
> Tomas
>
> On 05/22/2013 11:53 AM, Petr Jiricka wrote:
>> On May 22, 2013, at 11:32 AM, Tomas Kraus wrote:
>>
>>> The fix Petr H suggested yesterday is simple - change minimal pool
>>> size in tooling library to 0. I would also add shutdown call to close
>>> method. It's 2 lines change but it solves only part of the issue -
>>> hanging threads in memory.
>> The question is whether it fixes the *main* issue. We are in high
>> resistance mode, so only serious and severe issues should be fixed,
>> not P3-grade issues - especially at the risk of introducing regressions.
>>
>>> Log readers will still be interrupted in build task.
>>>
>>> But it's up to you. I'll do one more release branch clone to prepare
>>> this version.
>> If the simple fix fixes the main issue, and everyone agrees that this
>> change is correct and safe, then let's do it this way.
>>
>> Thanks,
>> Petr
>>
>>
>>> Tooling Library changes don't have to be removed because new code
>>> won't be used.
>>>
>>> Tomas
>>>
>>> On 05/22/2013 11:25 AM, Petr Jiricka wrote:
>>>> On May 21, 2013, at 9:58 PM, Petr Hejl wrote:
>>>>
>>>>> Hi,
>>>>> I'm sorry, but your formulation looks like I'm ok with the patch.
>>>>>
>>>>> What I don't like about the patch? It is a bit large and does fixes
>>>>> in area which is hard and error prone on its own (threading). It
>>>>> also fixes a lot more than just thread leak, while there is simpler
>>>>> solution to the leak itself. We already found one issue in the
>>>>> original patch and looking at the diff there is another one in the
>>>>> library - not synchronized access to eventListeners in
>>>>> notifyListeners.
>>>> In the light of this, it is clear that this fix is too risky - let's
>>>> not put it into 7.3.1.
>>>>
>>>> Tomas, is there a simpler fix that would fix the main problem and
>>>> that would be non-controversial? I believe Petr H suggested an
>>>> uncomplicated fix, could we use that? If we do, then we should give
>>>> QA a jar file with the fix for testing with 7.3.1 builds.
>>>>
>>>> Thanks,
>>>> Petr
>>>>
>>>>
>>>>> So the whole thing is a little bit risky from my point of view.
>>>>>
>>>>> I have no objections about the integration if other reviewers are
>>>>> happy about the patch.
>>>>> P.
>>>>>
>>>>> On 05/21/2013 01:42 PM, Tomas Kraus wrote:
>>>>>> *Bug 228886*<https://netbeans.org/bugzilla/show_bug.cgi?id=228886>
>>>>>> -Multiple Glassfish threads hanging in memory
>>>>>>
>>>>>> Zombie thread is left after GlassFish Log Reader thread has been
>>>>>> interrupted in build task.
>>>>>>
>>>>>> - Start NetBeans
>>>>>> - Start VisualVM and open NetBeans, threads table, sort by Thread
>>>>>> - Create sample project ServletStateless
>>>>>> - Start GlassFish server in debug mode
>>>>>> - Undeploy all projects
>>>>>> Repeat the following cycle
>>>>>> - Run project ServletStateless
>>>>>>    creates one thread FetchLogPiped
>>>>>> - Clean and build project
>>>>>>    creates one thread FetchLogPiped.
>>>>>>
>>>>>> So if you run this cyle 3 times you should have 6 FetchLogPiped
>>>>>> threads.
>>>>>>
>>>>>> Fix in GF plugin was made in 4 steps (253015:70350e3b09dc,
>>>>>> 253069:422df5694675, 253094:1ad1e811d955 and 253176:e0be6fab29ed),
>>>>>> combined diff is attached as
>>>>>> https://netbeans.org/bugzilla/attachment.cgi?id=134564&action=diff
>>>>>>
>>>>>> I made single thread pool (ExecutorService) to be shared for all
>>>>>> GlassFish Log Reader threads. Threads are run under top level
>>>>>> ThreadGroup to not be interrupted by build task. Threads have
>>>>>> setDaemon(true) set to avoid them being NB exit blockers.
>>>>>>
>>>>>> GlassFish Tooling Library changeset is 502:280234be9cb0.
>>>>>>
>>>>>> It allows to pass external ExecutorService into GlassFish Log Reader
>>>>>> thread starting code.
>>>>>>
>>>>>> Code went trough 2 reviews - Petr Hejl and Peter Benedikovic. It was
>>>>>> cleaned up thanks to Petr Hejl notes and now I believe it's safe
>>>>>> enough
>>>>>> to be pushed in to 7.3.1 release branch. QA testing was done by Jiri
>>>>>> Skrivanek. More details are in the bug.
>>>>>>
>