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
> _______________________________________________
> Geoserver-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>
>
------------------------------------------------------------------------------
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
