Thank you! (fyi, my issue is not related)

On Thu, Sep 18, 2014 at 9:08 AM, Rob Walker <[email protected]> wrote:

>  Well, correct behaviour would be to not close the stream multiple times.
> Personally, I think it's bad form for a block of code to issue a close() on
> a stream that it didn't open, it's not symmetrical.  But that's not always
> as easy to avoid as it sounds. In this case, completely avoiding a double
> close may not be possible since there is a suspicion at least one of the
> close calls is happening inside Jetty. At least that is my understanding, I
> wasn't the one who found the original issue.
>
> The problem with not ignoring the close exception, is that without that
> try {} catch block the duplicate close causes a fatal exception which
> prevents HTTP based bundle loading from working. The only exceptions we've
> ever trapped inside this block is the duplicate close - of course others
> with different topologies may be able to trigger other close() exceptions.
>
> For us, we see the double close happening regularly when our server is
> significantly remote from teh instance we are web starting e.g. the Felix
> based code server is in our office, and the instance being launched over
> Web Start is on an AWS cloud server. In this setup, we get regular launch
> failures which prevent the application launching. So ignoring the duplicate
> close is definitely the lesser of two evils!
>
> *For Raymond: stack trace attached*
>
> *We're on a pretty old version of Jetty (6.1.26 I think). We have a task
> to update this at some stage, but when it works fine you tend to leave it.
> Note that the JarRevision code hasn't changed as far as I can see, so if
> Jetty behaviour is different in this area I suspect the problem will still
> be lurking even in newer versions*
>
> -- Rob
>
>
>
> On 18/09/2014 14:35, Richard S. Hall wrote:
>
> I guess the question to ask is what behavior is the correct behavior if
> there really is an exception on close? This patch will ignore every
> exception, not just ones that occurs because of a double close (although it
> seems to me a double close should be a no-op, not sure why it throws, but
> that is another issue).
>
> Does it make sense that every close exception is ignored and the user is
> never alerted? Granted, there isn't much we can do, but often such errors
> are better breaking things than just being ignored.
>
> -> richard
>
> On 9/18/14 03:43 , Rob Walker wrote:
>
> Just wanted to check this one before I raise a JIRA, in case it's known
> and/or already been fixed or being worked on.
>
> In one of our models we  provision our application over WebStart, which
> causes bundles to be loaded over HTTP. Occasionally we get a Stream Closed
> exception being thrown from the Jetty HTTP server (the server serving up
> the code is also running under Felix).
>
> What seems to happen is a double close of the input stream - which throws
> an error. I've include the notes below from our developer who found this.
>
> It's a pretty simple mod to ignore the exception on second close
> (highlighted red below) - assuming everyone is happy this is an ok
> approach.
>
> -- Rob
>
> ===
>
> /It turns out the problem was arising when caching the VersaTest jar
> files. Whilst they are coming across via WebStart ok, an error was being
> thrown because the stream used to read them was being closed multiple times
> and at some point the close operation causes an exception that isn't caught
> so stops the load.  This error occurs in Felix in the JarRevision class, in
> the initialize() method. I put a simple fix in that ignored the error on
> close. So in the code below, a close on the input stream was being called
> in the //BundleCache.copyStreamToFile(is, m_bundleFile)  and in the bottom
> finally block (you can see below where I put in that disregarding catch.
> (Note, I suspect that also the input stream could be closed in the
> //((HttpURLConnection) conn).disconnect() //as well. But it is explicitly
> closed in the copyStreamToFile.)////
> ///
>
>                 if (!byReference)
>                 {
>                     URLConnection conn = null;
>                     try
>                     {
>                         if (is == null)
>                         {
>                             // Do it the manual way to have a chance to
>                             // set request properties such as proxy auth.
>                             URL url =
> BundleCache.getSecureAction().createURL(
>                                 null, getLocation(), null);
>                             conn = url.openConnection();
>
>                             // Support for http proxy authentication.
>                             String auth = BundleCache.getSecureAction()
> .getSystemProperty("http.proxyAuth", null);
>                             if ((auth != null) && (auth.length() > 0))
>                             {
>                                 if ("http".equals(url.getProtocol()) ||
> "https".equals(url.getProtocol()))
>                                 {
>                                     String base64 =
> Util.base64Encode(auth);
> conn.setRequestProperty(
> "Proxy-Authorization", "Basic " + base64);
>                                 }
>                             }
>                             is = BundleCache.getSecureAction()
> .getURLConnectionInputStream(conn);
>                         }
>
>                         // Save the bundle jar file.
> BundleCache.copyStreamToFile(is, m_bundleFile);
>                     }
>                     finally
>                     {
>                         // This is a hack to fix an issue on Android,
> where
>                         // HttpURLConnections are not properly closed.
> (FELIX-2728)
>                         if ((conn != null) && (conn instanceof
> HttpURLConnection))
>                         {
>                             ((HttpURLConnection) conn).disconnect();
>                         }
>                     }
>                 }
>             }
>         }
>         finally
>         {
>             if (is != null)
>             {
>                 try
>                 {
>                     is.close();
>                 } catch (IOException ioe)
>                 {
>                     /*  Catch and disregard exception on close as could
> already have been closed,
>                         either in the disconnect or in the
> copyStreamToProfile
>                     */
>
>                 }
>
>             }
>         }
>
>
>
>
>
>
>
>
>
>
>
>
>
> --
>
>
> Ascert - Taking systems to the [email protected]
> SA +27 21 300 2028
> UK +44 20 7488 3470 ext 5119www.ascert.com
>
>


-- 
*Raymond Augé* <http://www.liferay.com/web/raymond.auge/profile>
 (@rotty3000)
Senior Software Architect
*Liferay, Inc.* <http://www.liferay.com> (@Liferay)

Reply via email to