You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@activemq.apache.org by mtaylor <gi...@git.apache.org> on 2016/01/11 18:35:12 UTC

[GitHub] activemq-artemis pull request: Remove DelegatingSession class

GitHub user mtaylor opened a pull request:

    https://github.com/apache/activemq-artemis/pull/308

    Remove DelegatingSession class

    DelegatingSession class wraps ClientSessionImpl and attempts to close
    session should it not be closed by the user.  It does this by
    implementing finalize.  However, the order in which finalize runs can be
    difficult to predict as compilers, and JIT compilers are able to
    optimize early.
    
    The current DelegatingSession was causing problems of finalize getting
    called early (before consumers, producers were finished with the
    session).  This was causing tests to fail on the IBM JDK (which
    optimizes early).  The same happens on OpenJDK if the GC is forced.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mtaylor/activemq-artemis Finalize

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/308.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #308
    
----
commit ae4921c5f6724aa817de6d4bd531fcede29dfc52
Author: Martyn Taylor <mt...@redhat.com>
Date:   2016-01-11T11:15:39Z

    Remove DelegatingSession class
    
    DelegatingSession class wraps ClientSessionImpl and attempts to close
    session should it not be closed by the user.  It does this by
    implementing finalize.  However, the order in which finalize runs can be
    difficult to predict as compilers, and JIT compilers are able to
    optimize early.
    
    The current DelegatingSession was causing problems of finalize getting
    called early (before consumers, producers were finished with the
    session).  This was causing tests to fail on the IBM JDK (which
    optimizes early).  The same happens on OpenJDK if the GC is forced.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: Remove DelegatingSession class

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/308


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: Remove DelegatingSession class

Posted by clebertsuconic <gi...@git.apache.org>.

Github user clebertsuconic commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/308#issuecomment-170684293
  
    I have sent an improved fix here: https://github.com/mtaylor/activemq-artemis/pull/1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: Remove DelegatingSession class

Posted by clebertsuconic <gi...@git.apache.org>.

Github user clebertsuconic commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/308#issuecomment-170744921
  
    never mind my PR. I could test it on the different JDKs and saw issues.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: Remove DelegatingSession class

Posted by clebertsuconic <gi...@git.apache.org>.

Github user clebertsuconic commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/308#issuecomment-170631396
  
    can you run the full testsuite before we merge this? let me know when you finished. it's sensitive area on the tests.. we have to do but better manage where it's breaking first


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---