Hi Jody,
wanted to check if you want to review them (seems like you had a quick look
already)
or I should just go ahead and merge?
Cheers
Andrea
PS: in case of unclosed streams the logged warning states which sysvar
should be set (sparing one
to have to look for it, which is always annoying for its sibling
gt.jdbc.trace one)
On Tue, Apr 12, 2016 at 6:07 PM, Jody Garnett <[email protected]>
wrote:
> Thanks Andrea, especially for the leak warning and reporting (seems to be
> -Dgs.lock.trace=true).
>
> --
> Jody Garnett
>
> On 12 April 2016 at 03:50, Andrea Aime <[email protected]>
> wrote:
>
>> Hi Jody,
>> I've made a couple of pull requests for master and 2.8.x, changing the
>> default lock provider, fixing the input
>> stream leaks, and since not closing streams can now cause temporary
>> server lockout, I've also added
>> jdbc connection style leak warning and reporting, with a system variable
>> that can be setup to locate where
>> the leak is coming from.
>>
>> https://github.com/geoserver/geoserver/pull/1563 (master)
>> https://github.com/geoserver/geoserver/pull/1564 (2.8.x)
>>
>> Cheers
>> Andrea
>>
>>
>> On Tue, Apr 12, 2016 at 7:55 AM, Jody Garnett <[email protected]>
>> wrote:
>>
>>> Good hunting, we can of course make the null lock provider the default
>>> (and should probably due so for the 2.9 release). Long term I would like to
>>> fix all the references to in()
>>>
>>> --
>>> Jody Garnett
>>>
>>> On 11 April 2016 at 11:09, Andrea Aime <[email protected]>
>>> wrote:
>>>
>>>> Hi,
>>>> I was looking at a jstack output provided by a colleague of mine that
>>>> managed to get himself locked out of the configuration UI and... well...
>>>> from what I can see, it's not hard at all to get the admin locked out of
>>>> the UI, and/or not being able to modify any of it, at least for some
>>>> time.
>>>>
>>>> Looking at the FileSystemResource implementation we have:
>>>>
>>>> @Override
>>>> public InputStream in() {
>>>> File actualFile = file();
>>>> if (!actualFile.exists()) {
>>>> throw new IllegalStateException("File not found " +
>>>> actualFile);
>>>> }
>>>> final Lock lock = lock();
>>>> try {
>>>> return new FileInputStream(file) {
>>>> @Override
>>>> public void close() throws IOException {
>>>> super.close();
>>>> lock.release();
>>>> }
>>>> };
>>>> } catch (FileNotFoundException e) {
>>>> lock.release();
>>>> throw new IllegalStateException("File not found " +
>>>> actualFile, e);
>>>> }
>>>> }
>>>>
>>>> So, if the input stream there is not closed, then the lock is not going
>>>> to be released,
>>>> not until garbage collection is triggered.
>>>>
>>>> How common is to grab a in() and not close it? Common, I believe I've
>>>> fixed at least
>>>> a couple such occurrences in the last months, just stumbling by
>>>> accident on them.
>>>> I made a quick check, Eclipse can see 35 calls of that in() method,
>>>> spotted a few hwere it's not closed, which means, has the lock that's
>>>> not released until
>>>> the finalizer calls close(),
>>>>
>>>> One is is logging logging (which is fixed on master btw, but not in
>>>> 2.8.x):
>>>>
>>>> https://github.com/geoserver/geoserver/blob/2.8.x/src/main/src/main/java/org/geoserver/logging/LoggingUtils.java#L180
>>>>
>>>> One parsing styles, because the reader is turned eventually into an
>>>> InputSource, which does not even
>>>> have a close method:
>>>>
>>>> https://github.com/geoserver/geoserver/blob/master/src/main/src/main/java/org/geoserver/catalog/StyleHandler.java#L192
>>>>
>>>> There might be others in code that I have not available in Eclipse,
>>>> yet, it's a worrisome since
>>>> a common resource management error can get to a short lived lockdown
>>>> (how short, depends on gc
>>>> and finalization, which are not predictable).
>>>>
>>>> When that happens however all GUI and REST access goes bye bye, because
>>>> those calls are grabbing
>>>> the global configuration lock (our default catalog facade is not thread
>>>> safe, remember?), then they get
>>>> stuck on the resource own lock, and until that gets released, the
>>>> global config lock is also kept, meaning
>>>> every other admin type call is blocked too...
>>>>
>>>> Before the resource refactor that was not much a big of a deal, since
>>>> there was only the global config lock,
>>>> and stream mis-management was not going to cause big issues (well, at
>>>> least not on linux).
>>>>
>>>> It's also making me wonder a bit about the resource level locks...
>>>> given that we have already the
>>>> global resource lock, which is not going away until we can rewrite the
>>>> default catalog facade (and maybe more,
>>>> what we'd really need it transactional memory...), is it worth having
>>>> also a resource level lock for stand-alone GeoServers?
>>>>
>>>> I have the impression that if the global config lock is not disabled
>>>> (there is a sysvar to do that) we'd
>>>> better default on a null lock provider, instead of an in memory one
>>>> like today?
>>>>
>>>> https://github.com/geoserver/geoserver/blob/master/src/main/src/main/java/org/geoserver/config/LockProviderInitializer.java#L53
>>>>
>>>> The current two levels of lock are redundant for a stand alone server,
>>>> and are actually causing trouble when
>>>> resource input streams are not closed properly...
>>>>
>>>> Opinions? I believe we need at a solid but quick fix for this one, the
>>>> stable series it too prone to lock itself up
>>>> at the moment, and possibly a plan for a better fix in the future.
>>>>
>>>> Cheers
>>>> Andrea
>>>>
>>>>
>>>>
>>>> --
>>>> ==
>>>> GeoServer Professional Services from the experts! Visit
>>>> http://goo.gl/it488V for more information.
>>>> ==
>>>>
>>>> Ing. Andrea Aime
>>>> @geowolf
>>>> Technical Lead
>>>>
>>>> GeoSolutions S.A.S.
>>>> Via di Montramito 3/A
>>>> 55054 Massarosa (LU)
>>>> phone: +39 0584 962313
>>>> fax: +39 0584 1660272
>>>> mob: +39 339 8844549
>>>>
>>>> http://www.geo-solutions.it
>>>> http://twitter.com/geosolutions_it
>>>>
>>>> *AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
>>>>
>>>> Le informazioni contenute in questo messaggio di posta elettronica e/o
>>>> nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
>>>> loro utilizzo è consentito esclusivamente al destinatario del messaggio,
>>>> per le finalità indicate nel messaggio stesso. Qualora riceviate questo
>>>> messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
>>>> darcene notizia via e-mail e di procedere alla distruzione del messaggio
>>>> stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
>>>> divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
>>>> utilizzarlo per finalità diverse, costituisce comportamento contrario ai
>>>> principi dettati dal D.Lgs. 196/2003.
>>>>
>>>>
>>>>
>>>> The information in this message and/or attachments, is intended solely
>>>> for the attention and use of the named addressee(s) and may be confidential
>>>> or proprietary in nature or covered by the provisions of privacy act
>>>> (Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
>>>> Code).Any use not in accord with its purpose, any disclosure, reproduction,
>>>> copying, distribution, or either dissemination, either whole or partial, is
>>>> strictly forbidden except previous formal approval of the named
>>>> addressee(s). If you are not the intended recipient, please contact
>>>> immediately the sender by telephone, fax or e-mail and delete the
>>>> information in this message that has been received in error. The sender
>>>> does not give any warranty or accept liability as the content, accuracy or
>>>> completeness of sent messages and accepts no responsibility for changes
>>>> made after they were sent or for other risks which arise as a result of
>>>> e-mail transmission, viruses, etc.
>>>>
>>>> -------------------------------------------------------
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Find and fix application performance issues faster with Applications
>>>> Manager
>>>> Applications Manager provides deep performance insights into multiple
>>>> tiers of
>>>> your business applications. It resolves application problems quickly and
>>>> reduces your MTTR. Get your free trial!
>>>> http://pubads.g.doubleclick.net/
>>>> gampad/clk?id=1444514301&iu=/ca-pub-7940484522588532
>>>> <http://pubads.g.doubleclick.net/gampad/clk?id=1444514301&iu=/ca-pub-7940484522588532>
>>>> _______________________________________________
>>>> Geoserver-devel mailing list
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>>>>
>>>>
>>>
>>
>>
>> --
>> ==
>> GeoServer Professional Services from the experts! Visit
>> http://goo.gl/it488V for more information.
>> ==
>>
>> Ing. Andrea Aime
>> @geowolf
>> Technical Lead
>>
>> GeoSolutions S.A.S.
>> Via di Montramito 3/A
>> 55054 Massarosa (LU)
>> phone: +39 0584 962313
>> fax: +39 0584 1660272
>> mob: +39 339 8844549
>>
>> http://www.geo-solutions.it
>> http://twitter.com/geosolutions_it
>>
>> *AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
>>
>> Le informazioni contenute in questo messaggio di posta elettronica e/o
>> nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
>> loro utilizzo è consentito esclusivamente al destinatario del messaggio,
>> per le finalità indicate nel messaggio stesso. Qualora riceviate questo
>> messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
>> darcene notizia via e-mail e di procedere alla distruzione del messaggio
>> stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
>> divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
>> utilizzarlo per finalità diverse, costituisce comportamento contrario ai
>> principi dettati dal D.Lgs. 196/2003.
>>
>>
>>
>> The information in this message and/or attachments, is intended solely
>> for the attention and use of the named addressee(s) and may be confidential
>> or proprietary in nature or covered by the provisions of privacy act
>> (Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
>> Code).Any use not in accord with its purpose, any disclosure, reproduction,
>> copying, distribution, or either dissemination, either whole or partial, is
>> strictly forbidden except previous formal approval of the named
>> addressee(s). If you are not the intended recipient, please contact
>> immediately the sender by telephone, fax or e-mail and delete the
>> information in this message that has been received in error. The sender
>> does not give any warranty or accept liability as the content, accuracy or
>> completeness of sent messages and accepts no responsibility for changes
>> made after they were sent or for other risks which arise as a result of
>> e-mail transmission, viruses, etc.
>>
>> -------------------------------------------------------
>>
>
>
--
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via di Montramito 3/A
55054 Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento contrario ai
principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely for
the attention and use of the named addressee(s) and may be confidential or
proprietary in nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure, reproduction,
copying, distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please contact
immediately the sender by telephone, fax or e-mail and delete the
information in this message that has been received in error. The sender
does not give any warranty or accept liability as the content, accuracy or
completeness of sent messages and accepts no responsibility for changes
made after they were sent or for other risks which arise as a result of
e-mail transmission, viruses, etc.
-------------------------------------------------------
------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
