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

memory leak using c3p0 pooling library #83

Open
randallt opened this issue Aug 20, 2020 · 3 comments · May be fixed by #97
Open

memory leak using c3p0 pooling library #83

randallt opened this issue Aug 20, 2020 · 3 comments · May be fixed by #97

Comments

@randallt
Copy link

randallt commented Aug 20, 2020

I am using a custom fork of the Java Special Agent. We have a service that relies on the Quartz scheduler, which uses c3p0 (v0.9.1.1). With the tracing agent applied, the service leaks memory until it throws an OOM error.

A heap dump showed that TracingPreparedStatements were being leaked. I tracked this to the use of the WrapperProxy and the fact that, given a Proxy instance of TracingConnection or TracingStatement, a call to obj.equals(obj) will always return false. This is because the WrapperProxy is delegating all method calls to the wrapper or the original object, but these don't know how to handle the added Proxy/WrapperProxy layers for the equals method. This leads to c3p0 not being able to clean up its collections for the connections/statements.

I was able to fix this by adding a custom equals method to TracingStatement:

  @Override
  public boolean equals(Object obj) {
	if (this == obj) {
		return true;
	} else if (obj == null) {
		return false;
	} else if (WrapperProxy.isWrapper(obj, getClass())) {
		return obj.equals(this); // trick to get through the WrapperProxy/Proxy instances
	} else if (getClass() != obj.getClass()) {
		return false;
	} else {
		TracingStatement other = (TracingStatement) obj;
		if (statement == null) {
			if (other.statement != null) {
				return false;
			}
		} else if (statement != other.statement && !statement.equals(other.statement)) {
			return false;
		}
	}
	return true;
  }

and to TracingConnection:

  @Override
  public boolean equals(Object obj) {
	if (this == obj) {
		return true;
	} else if (obj == null) {
		return false;
	} else if (WrapperProxy.isWrapper(obj, getClass())) {
		return obj.equals(this); // trick to get through the WrapperProxy/Proxy instances
	} else if (getClass() != obj.getClass()) {
		return false;
	} else {
		TracingConnection other = (TracingConnection) obj;
		if (connection == null) {
			if (other.connection != null) {
				return false;
			}
		} else if (connection != other.connection && !connection.equals(other.connection)) {
			return false;
		}
	}
	return true;
  }

These may not be 100% hardened, but they got me past the memory leak with c3p0. I thought I should be a good citizen and open an issue regarding this.

@randallt randallt changed the title memory leak using C3PO pooling library memory leak using c3p0 pooling library Aug 21, 2020
@malafeev
Copy link
Contributor

@randallt would you like to submit PR? please add comments to the code.

@randallt
Copy link
Author

Can someone please submit the PR for me? The only non-standard thing about the equals method is the trick to call the equals method on 'obj', which gets through the Proxy and WrapperProxy layers so that one of the other comparisons can function.

@quaff quaff linked a pull request Sep 14, 2020 that will close this issue
@quaff
Copy link
Contributor

quaff commented Sep 14, 2020

@randallt could you test #97

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants