WorkImpl may leak an InboundJmsResource

[glassfishrobot]

This is a companion to #57 and was found by code inspection whilst investigating the former issue.

com.sun.genericra.inbound.async.WorkImpl.run() contains a code path which neglects to release the ServerSession (= InboundJmsResource) instance back to its pool, viz. when an exception is thrown whilst releasing the message endpoint. This would result in leaking one resource from the pool, leaving it unusable until the application server is restarted. If the MaxPoolSize is 1, or otherwise if enough resources have been leaked in this way, messages from the queue associated with this pool would no longer be processed.

I have never actually seen this happen, and it is unlikely to go unnoticed when it happens, since said exception would be logged at level SEVERE. But it is easy to fix.

Environment

GlassFish (2.1.1 or 3.1.2), Linux (SLES10SP2, SLES11SP2), x86_64

Affected Versions

[0.9, 1.0a, 1.0, v1.5, 1.5, v1.6, 1.6, 2.1a, 2.1b, current, 2.2a]

[glassfishrobot]

• Issue Imported From: https://github.com/javaee/glassfish-genericjmsra/issues/58
• Original Issue Raised By:@glassfishrobot
• Original Issue Assigned To: @glassfishrobot

[glassfishrobot]

@glassfishrobot Commented Reported by gerhard_niklasch

[glassfishrobot]

@glassfishrobot Commented gerhard_niklasch said: Proposed fix (along with minor whitespace clean-up):

— ../../WorkImpl.java,rev=236 2013-12-13 13:37:05.000000000 +0100 +++ com/sun/genericra/inbound/async/WorkImpl.java 2013-12-13 13:41:04.000000000 +0100 @@ -1,5 +1,5 @@ /**

• • Copyright (c) 2004-2005 Oracle and/or its affiliates. All rights reserved.
  • • Copyright (c) 2004-2013 Oracle and/or its affiliates. All rights reserved.

• Licensed under the Apache License, Version 2.0 (the "License");

• you may not use this file except in compliance with the License. @@ -35,17 +35,17 @@ */ public class WorkImpl implements Work { private static Logger _logger;

• + static

  { _logger = LogUtils.getLogger(); }

• + InboundJmsResource jmsResource;

• + public WorkImpl(InboundJmsResource jmsResource)

  { this.jmsResource = jmsResource; }

• + public void run() { try

  { _logger.log(Level.FINER, "Now running the message consumption"); @@ -54,24 +54,26 @@ }

  catch (Exception e)

  { e.printStackTrace(); }

  finally {

• try {

  • try { this.jmsResource.releaseEndpoint(); DeliveryHelper helper = this.jmsResource.getDeliveryHelper(); if (helper.markedForDMD()) { helper.sendMessageToDMD(); }
• • } catch (Exception e)

  { + _logger.log(Level.SEVERE, + "Exception while releasing the JMS endpoint" + e.getMessage()); + }

  • try

  { this.jmsResource.release(); }

  catch (Exception e)

  { - _logger.log(Level.SEVERE, + _logger.log(Level.SEVERE, "Exception while releasing the JMS resource" + e.getMessage()); }

• _logger.log(Level.FINER, "Freed the resource now"); } }

• + public void release()

  { // For now do nothing. }

(Contact me via the dev mailing list for the literal version, since the JIRA web interface tends to mangle the context-diff output and doesn't allow me to upload it as an attachment.)

[glassfishrobot]

@glassfishrobot Commented gerhard_niklasch said: This is now fixed in trunk revision 237 as part of the putback for Bug#17973448.

[glassfishrobot]

@glassfishrobot Commented This issue was imported from java.net JIRA GENERICJMSRA-58