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

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 scikit-learn#8499
@agramfort
Copy link
Member

thx

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

@superbobry
Copy link
Contributor Author

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

@agramfort
Copy link
Member

agramfort commented Jun 6, 2017 via email

@jnothman
Copy link
Member

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
@jnothman
Copy link
Member

jnothman commented Jun 7, 2017

Thanks @superbobry

@vene
Copy link
Member

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 commented Jun 7, 2017

Oh :/

@vene
Copy link
Member

vene commented Jun 7, 2017

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

Sundrique pushed 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 pushed 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 pushed 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 pushed 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 pushed 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 pushed 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 pushed 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leak in LogisticRegression
4 participants