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

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.
>=20
> 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.
>=20
> Tomas
>=20
> On 05/22/2013 11:25 AM, Petr Jiricka wrote:
>> On May 21, 2013, at 9:58 PM, Petr Hejl wrote:
>>=20
>>> Hi,
>>> I'm sorry, but your formulation looks like I'm ok with the patch.
>>>=20
>>> 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.
>>=20
>> 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.
>>=20
>> Thanks,
>> Petr
>>=20
>>=20
>>> So the whole thing is a little bit risky from my point of view.
>>>=20
>>> I have no objections about the integration if other reviewers are =
happy about the patch.
>>> P.
>>>=20
>>> On 05/21/2013 01:42 PM, Tomas Kraus wrote:
>>>> *Bug 228886*<https://netbeans.org/bugzilla/show_bug.cgi?id=3D228886>
>>>> -Multiple Glassfish threads hanging in memory
>>>>=20
>>>> Zombie thread is left after GlassFish Log Reader thread has been
>>>> interrupted in build task.
>>>>=20
>>>> - 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.
>>>>=20
>>>> So if you run this cyle 3 times you should have 6 FetchLogPiped =
threads.
>>>>=20
>>>> 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=3D134564&action=3Ddif=
f
>>>>=20
>>>> 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.
>>>>=20
>>>> GlassFish Tooling Library changeset is 502:280234be9cb0.
>>>>=20
>>>> It allows to pass external ExecutorService into GlassFish Log =
Reader
>>>> thread starting code.
>>>>=20
>>>> 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.
>>>>=20
>=20