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

Workaround for #1 #2

Merged
merged 1 commit into from
Jan 18, 2012
Merged

Workaround for #1 #2

merged 1 commit into from
Jan 18, 2012

Conversation

kennethreitz
Copy link
Contributor

So, apparently sometimes getrefcount is None. This was causing the strange hidden exception behavior.

When an exception occurs in __del__ methods, they get suppressed and printed to the terminal upon process exit.

This simple try/except makes my problem go away, but is prob not the best way to fix it. :)

@Byron
Copy link
Member

Byron commented Jul 8, 2011

Thank you !
What I don't like about it is that it fixes the symptoms,but not the cause. Why is it returning None in the first place ? If it returns None, the whole system of using the internal refcount doesn't really work anymore, which worries me.

In a way, this fix just silences the error popping up in the destructor, but its not a fix.
Unfortunately, I have no idea why it happens.

Soon I will have osx lion, so I should be able to reproduce it.

@kennethreitz
Copy link
Contributor Author

Absolutely. I know very little about refcount and smmap, else I would have investigated further :)

@kennethreitz
Copy link
Contributor Author

Any update on this?

This bug has been preventing me from releasing the next version of some software for 2 months now.

@Byron
Copy link
Member

Byron commented Aug 31, 2011

Thanks for bringing this up again, I didn't have it on my radar anymore.

I just ran the unittests on OSX Lion, and to my surprise they did all pass.

As I cannot reproduce the issue, I see no way of fixing the cause of the issue.
For you to proceed, I recommend using your version of smmap in your deployment. For the reasons mentioned previously, I am afraid I cannot merge your patch.

If you run the unittests with nosetest, they should fail as well or show errors of any kind - this should help you to nail down the issue and fix it at its roots. These roots don't necessarily lie within smmap, as getrefcount is a default method that is expected to exist in any python installation.

Good luck hunting !

@kennethreitz
Copy link
Contributor Author

I've tried to push up my own version to PyPi, but the setup.py throws up all over w/ extension issues.

When you deploy a new version, are you doing something other than python setup.py sdist upload?


Can you explain why smmap is a dependency of gitpython in the first place? Ideally, I see no reason for gitpython to not be pure python.

@Byron
Copy link
Member

Byron commented Sep 4, 2011

I don't think you can just push your own version of an existing project. What you could do though is to just include the code in your project directly.

smmap is used by gitdb, which in turn is used by gitpython. smmap is pure python, but gitdb has a little optional c code. Async is a dependency coming in with gitdb as well, and it too has some optional c code.

@kennethreitz
Copy link
Contributor Author

Dependency hell.

@kennethreitz
Copy link
Contributor Author

Did you ever get a chance to look at this? I'm still having this error — 5 months later.

Exception TypeError: "'NoneType' object is not callable" in <bound method WindowCursor.__del__ of <smmap.mman.WindowCursor object at 0x10d86e628>> ignored

@Byron
Copy link
Member

Byron commented Nov 9, 2011

I couldn't reproduce the issue, see #2 (comment) .

@Byron Byron closed this Nov 9, 2011
@kennethreitz
Copy link
Contributor Author

I think, sadly enough, that I'm going to have to formally fork GitPython to remove the metric fuckton of dependencies.

@Byron
Copy link
Member

Byron commented Nov 9, 2011

I am sorry I could be of no help for you, but if it helps to reduce your mental pain at least, please note that I am intending to reduce the dependencies in the latest release of GitPython. The goal there is to remove all of them except for smmap - currently it still uses smmap and async.

Getting your fix into GitPython should be as easy as forking it and using your smmap repository and revision for the smmap submodule in ext/smmap.
Depending on your distribution scheme you might have to adjust setup.py or the manifest in order to get a distribution which bakes the dependencies.

@Autoplectic
Copy link

so i did some looking in to this and the issue is more odd that it would seem.

the issue is not that getrefcount sometimes returns none, it's that sometimes it raises a typeerror... or at least seems that way. using print as a debugger the error seems to be raised during the getrefcount call.

now, the weird part... if you replace line 55 of mmap.py with:

num_clients = getrefcount(self._rlist) - 4

i think the 4 is right... the -2 that was there before and 2 of the 3 from the comment in client_count().

in any case, calling getrefcount directly in the _destroy method no longer causes an error to be raised.

i hope this is an acceptable solution... i know it doesn't find exactly why the error is being raised, but it does track down exactly where it is coming from, and removes it without hiding anything.

@kennethreitz
Copy link
Contributor Author

@Autoplectic: thanks for digging around and finding this!

@Autoplectic
Copy link

so slight update. even though mmap.py contains:

import sys
from sys import getrefcount

both are None within the _destroy method. not sure why. so anyway, just to be clear, if you change line 55 to the lines

from sys import getrefcount
num_clients = getrefcount(self._rlist) - 4

it works without error, and should logically be doing the same as using the call to client_count()

also, running smmap's nosetests results in no errors, so whatever error this is, it's not covered by the tests.

@Byron Byron reopened this Dec 5, 2011
@Byron
Copy link
Member

Byron commented Dec 5, 2011

Thanks for sharing your discovery !
I will make the change as soon as possible, or will merge your fix if you should be faster in providing a patch.

Thanks again !
Sebastian

@kennethreitz
Copy link
Contributor Author

Thanks a ton for this! Really looking forward to continuing development on legit now :)

@kennethreitz
Copy link
Contributor Author

Any update on when you can release a fix for this? I'd really like to start working on my git module again :)

@ghost ghost assigned Byron Jan 18, 2012
@Byron Byron merged commit e2b170a into gitpython-developers:master Jan 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants