Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a RefCounted interface and basic impl #8210

Merged
merged 1 commit into from Oct 24, 2014

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Oct 23, 2014

We already have two places duplicating this rather hairy logic, this
commit intorduces a new RefCoutned interace and an abstract implementation
that can be used for delegation. It factors out all the reference counting
and adds single and multithreaded test for it.

@s1monw
Copy link
Contributor Author

s1monw commented Oct 23, 2014

@bleskes all yours... ;)

@Override
public final void incRef() {
if (tryIncRef() == false) {
throw new AlreadyClosedException("Store is already closed can't increment refCount current count [" + refCount.get() + "]");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove the Store part. Perhaps make a constructor with a name? these errors are difficult tot trace so we should make it as clear as possible where the error came from (even if the stack trace is lost)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is a copy / paste error I will fix

@bleskes
Copy link
Contributor

bleskes commented Oct 24, 2014

LGTM. I like the abstract class + interface approach. Left two little comments.

@s1monw
Copy link
Contributor Author

s1monw commented Oct 24, 2014

@bleskes pushed a new commit

@bleskes
Copy link
Contributor

bleskes commented Oct 24, 2014

@s1monw looks great.

We already have two places duplicating this rather hairy logic, this
commit intorduces a new RefCoutned interace and an abstract implementation
that can be used for delegation. It factors out all the reference counting
and adds single and multithreaded test for it.

Closes elastic#8210
s1monw added a commit that referenced this pull request Oct 24, 2014
We already have two places duplicating this rather hairy logic, this
commit intorduces a new RefCoutned interace and an abstract implementation
that can be used for delegation. It factors out all the reference counting
and adds single and multithreaded test for it.

Closes #8210
@s1monw s1monw merged commit 347ce36 into elastic:master Oct 24, 2014
@s1monw s1monw deleted the refcounted branch October 24, 2014 08:45
@s1monw s1monw removed the review label Oct 24, 2014
@clintongormley clintongormley added the :Core/Infra/Core Core issues without another label label Mar 19, 2015
@clintongormley clintongormley changed the title [UTILITIES] Introduce a RefCounted interface and basic impl Introduce a RefCounted interface and basic impl Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants