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

[MRG+2] Fixed memory leak in liblinear #9024

Merged
merged 1 commit into from Jun 7, 2017

Conversation

@superbobry
Copy link
Contributor

@superbobry superbobry commented Jun 6, 2017

Reference Issue

Fixes #8499

What does this implement/fix? Explain your changes.

scikit-learn version of liblinear had tow leaks:

  • missing free of the problem struct
  • missing free of number of iterations

The former was present in the initial version of liblinear_helper.c
while latter appeared after c8c72fd
which introduced n_iter.

Any other comments?

I'm surprised these leaks survived for so long :)

The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes #8499
@agramfort
Copy link
Member

@agramfort agramfort commented Jun 6, 2017

thx

what's our policy here. Try to push upstream or we gave up on this?

@superbobry
Copy link
Contributor Author

@superbobry superbobry commented Jun 6, 2017

From what I can tell, both affected functions are sklearn-specific changes.

@agramfort
Copy link
Member

@agramfort agramfort commented Jun 6, 2017

@jnothman
Copy link
Member

@jnothman jnothman commented Jun 7, 2017

LGTM +1 for merge when green.

@jnothman jnothman changed the title [MRG] Fixed memory leak in liblinear [MRG+1] Fixed memory leak in liblinear Jun 7, 2017
@jnothman jnothman changed the title [MRG+1] Fixed memory leak in liblinear [MRG+2] Fixed memory leak in liblinear Jun 7, 2017
@jnothman jnothman merged commit e2f99b0 into scikit-learn:master Jun 7, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jnothman
Copy link
Member

@jnothman jnothman commented Jun 7, 2017

Thanks @superbobry

@vene
Copy link
Member

@vene vene commented Jun 7, 2017

triggered an circleci failure in building the docs: ./build_tools/circle/build_doc.sh died unexpectedly

@jnothman
Copy link
Member

@jnothman jnothman commented Jun 7, 2017

Oh :/

@vene
Copy link
Member

@vene vene commented Jun 7, 2017

aaand now it passes without being touched. Could have been a random failure...

Sundrique added a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes scikit-learn#8499
dmohns added a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017
The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes scikit-learn#8499
dmohns added a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017
The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes scikit-learn#8499
NelleV added a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes scikit-learn#8499
paulha added a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes scikit-learn#8499
AishwaryaRK added a commit to AishwaryaRK/scikit-learn that referenced this pull request Aug 29, 2017
The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes scikit-learn#8499
maskani-moh added a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes scikit-learn#8499
jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017
The leak resulted from two issues:
- not freeing the problem struct
- not freeing the number of iterations

The former was present in the initial version of ``liblinear_helper.c``
while latter appeared after c8c72fd
which introduced ``n_iter``.

Closes scikit-learn#8499
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.