Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Python: Bugs

[issue6615] multiprocessing logging support test

 

 

Python bugs RSS feed   Index | Next | Previous | View Threaded


report at bugs

Nov 21, 2009, 5:51 AM

Post #1 of 25 (822 views)
Permalink
[issue6615] multiprocessing logging support test

Changes by Ask Solem <askh [at] opera>:


----------
keywords: +patch
Added file: http://bugs.python.org/file15375/6615.patch

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 21, 2009, 6:38 AM

Post #2 of 25 (796 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Jesse Noller <jnoller [at] gmail> added the comment:

patch committed to trunk in r76438

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 21, 2009, 10:10 AM

Post #3 of 25 (791 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Jesse Noller <jnoller [at] gmail> added the comment:

merged to py3k in r76439

----------
resolution: -> fixed
status: open -> closed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 6:22 AM

Post #4 of 25 (756 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Jesse Noller <jnoller [at] gmail> added the comment:

I've commented out the test (therefore, reopening this) the test
introduces a pretty bad refleak problem. Need to debug.

----------
resolution: fixed -> accepted
status: closed -> open

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 6:27 AM

Post #5 of 25 (756 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Amaury Forgeot d'Arc <amauryfa [at] gmail> added the comment:

Are these leaks caused by the test, or revealed by it?

----------
nosy: +amaury.forgeotdarc

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 7:05 AM

Post #6 of 25 (756 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

>From a quick command-line attempt, it seems that logging is the culprit:

>>> handler = logging.Handler()
[64370 refs]
>>> handler = logging.Handler()
[64383 refs]
>>> handler = logging.Handler()
[64396 refs]

----------
nosy: +pitrou, vinay.sajip

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 7:14 AM

Post #7 of 25 (756 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Amaury Forgeot d'Arc <amauryfa [at] gmail> added the comment:

Yes, test_logging has a serious tearDown() method to wipe installed
handlers.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 7:16 AM

Post #8 of 25 (756 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> Yes, test_logging has a serious tearDown() method to wipe installed
> handlers.

In general it would be nice if logging were more conservative (or
cautious) when it comes to keeping objects persistent. It is too easy to
fall into a trap and experience memory leaks.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 8:40 AM

Post #9 of 25 (758 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Jesse Noller <jnoller [at] gmail> added the comment:

Yeah, I should have checked the tearDown stuff in the logging test suite

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 10:01 AM

Post #10 of 25 (756 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Vinay Sajip <vinay_sajip [at] yahoo> added the comment:

> In general it would be nice if logging were more conservative (or

> cautious) when it comes to keeping objects persistent. It is too easy to
> fall into a trap and experience memory leaks.

I've checked in a change into trunk today which might improve matters. Handler.__init__ no longer adds the handler instance to the _handlers map unconditionally, but still adds it to the _handlerList array. The only place this array is actually used is in the shutdown() API, which is registered to be run via atexit. It flushes and closes all the handlers in the list, so that any data in logging buffers gets flushed before the process exits.

The _handlers dict is now used to hold a mapping between handler names and handlers - the name property was added to Handler in the change I checked in today. This will be used by the dictConfig functionality which is proposed in PEP 391. If no name property is set on a Handler, then this will not add any additional references to handlers.

Python 2.7a0 (trunk:75403, Oct 14 2009, 20:14:09) [MSC v.1500 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import logging
[49139 refs]
>>> logging._handlerList
[]
[49146 refs]
>>> logging.Handler()
<logging.Handler object at 0x00B72658>
[49179 refs]
>>> logging.Handler()
<logging.Handler object at 0x00B72620>
[49202 refs]
>>> logging.Handler()
<logging.Handler object at 0x00B764D0>
[49225 refs]
>>> logging._handlerList
[.<logging.Handler object at 0x00B764D0>, <logging.Handler object at 0x00B72620>,
<logging.Handler object at 0x00B72658>]
[49225 refs]
>>> logging.shutdown()
[49156 refs]
>>> logging._handlerList
[]
[49156 refs]
>>>

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 10:08 AM

Post #11 of 25 (756 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> Handler.__init__ no longer adds the handler instance to the _handlers
> map unconditionally, but still adds it to the _handlerList array. The
> only place this array is actually used is in the shutdown() API, which
> is registered to be run via atexit. It flushes and closes all the
> handlers in the list, so that any data in logging buffers gets flushed
> before the process exits.

Why is this exactly? Why do you want to keep handlers until shutdown
rather than dispose of them when they aren't used anymore?

Moreover, closing in atexit is rather bad because the resources
necessary for proper closing (files, sockets...) may already have been
freed.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 10:23 AM

Post #12 of 25 (757 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Vinay Sajip <vinay_sajip [at] yahoo> added the comment:

> Why is this exactly? Why do you want to keep handlers until shutdown

> rather than dispose of them when they aren't used anymore?

Typically handlers are only instantiated when being added to loggers, and loggers live for the lifetime of the process so those handler references will typically be persistent. However, if a handler is closed (typically after removing from a handler), it's removed from the list and would not cause a memory leak.

> Moreover, closing in atexit is rather bad because the resources
> necessary for proper closing (files, sockets...) may already have been
> freed.

That's potentially true, but it's behaviour which has been there from the beginning so ISTM it needs to be there for backward compatibility reasons :-( When logging.raiseExceptions is True (the default) any exceptions in shutdown() would be raised, but this doesn't appear to happen in practice.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 10:24 AM

Post #13 of 25 (754 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Vinay Sajip <vinay_sajip [at] yahoo> added the comment:

> removing from a handler), it's removed from the list and would not cause a

s/handler/logger/

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 11:23 AM

Post #14 of 25 (753 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

flox <laxyf [at] yahoo> added the comment:

I would think to use weak references in order to prevent such leaks.

With the patch attached, if the handler is not referenced anywhere, it
is closed.
However named handlers are not closed (because of hard reference in
_handlers dictionary).

----------
nosy: +flox
Added file: http://bugs.python.org/file15395/issue6615_weakref.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 12:47 PM

Post #15 of 25 (746 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Vinay Sajip <vinay_sajip [at] yahoo> added the comment:

> With the patch attached, if the handler is not referenced anywhere, it

> is closed.
> However named handlers are not closed (because of hard reference in
> _handlers dictionary).

I haven't tried it yet, but does the patch actually work? You seem to have self_weakref in one place and self._weakref in another...

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 1:54 PM

Post #16 of 25 (747 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Changes by flox <laxyf [at] yahoo>:


Removed file: http://bugs.python.org/file15395/issue6615_weakref.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 2:41 PM

Post #17 of 25 (743 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

flox <laxyf [at] yahoo> added the comment:

yep... patch was not clean. Sorry :(

I changed it.
It passes the 21 tests of the test_logging suite.

And the count of references decreases with the test:

>>> import logging
>>> handler = logging.Handler()
>>> handler = logging.Handler()

----------
Added file: http://bugs.python.org/file15396/issue6615_weakref.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 2:46 PM

Post #18 of 25 (744 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

Some quick comments on your patch (not an in-depth review):
- you should add some tests for the problem you're trying to solve
- using __del__ when you have a weakref is counter-productive; use the
weakref's optional callback instead
- if you remove arbitrary elements from it, _handlerList should probably
be a set rather a list (but it's more of an optimization concern)
- `for h in [wr() for wr in handlerList if wr() is not None]` isn't a
pretty notation; just put the `if` inside the `for` instead

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 3:55 PM

Post #19 of 25 (750 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

flox <laxyf [at] yahoo> added the comment:

Updated the patch with technical recommendations from Antoine.

I kept the _handlerList as a list because "It allows handlers to be
removed in reverse of order initialized."

And some tests are needed to outline the change.

----------
Added file: http://bugs.python.org/file15397/issue6615_weakref.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 3:57 PM

Post #20 of 25 (743 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Changes by flox <laxyf [at] yahoo>:


Removed file: http://bugs.python.org/file15396/issue6615_weakref.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 4:14 PM

Post #21 of 25 (743 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

flox <laxyf [at] yahoo> added the comment:

Small change to acquire the module lock before working on _handlerList.

----------
Added file: http://bugs.python.org/file15398/issue6615_weakref.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 24, 2009, 4:14 PM

Post #22 of 25 (743 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Changes by flox <laxyf [at] yahoo>:


Removed file: http://bugs.python.org/file15397/issue6615_weakref.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 25, 2009, 1:38 AM

Post #23 of 25 (718 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Vinay Sajip <vinay_sajip [at] yahoo> added the comment:

Changes checked into trunk (r76508) - very slightly different to flox's patch. Also made _handlers a WeakValueDictionary.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 25, 2009, 2:45 AM

Post #24 of 25 (716 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

flox <laxyf [at] yahoo> added the comment:

Thank you Vinay.

Since you "reversed()" the _handlerList, the following part need to be
changed:

Index: Lib/logging/__init__.py
===================================================================
--- Lib/logging/__init__.py (revision 76508)
+++ Lib/logging/__init__.py (working copy)
@@ -610,7 +610,7 @@
"""
_acquireLock()
try:
- _handlerList.insert(0, weakref.ref(handler, _removeHandlerRef))
+ _handlerList.append(weakref.ref(handler, _removeHandlerRef))
finally:
_releaseLock()

----------
versions: +Python 2.7, Python 3.1, Python 3.2

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Nov 25, 2009, 6:13 AM

Post #25 of 25 (717 views)
Permalink
[issue6615] multiprocessing logging support test [In reply to]

Vinay Sajip <vinay_sajip [at] yahoo> added the comment:

> Since you "reversed()" the _handlerList, the following part need to be

> changed:
>
> - _handlerList.insert(0, weakref.ref(handler, _removeHandlerRef))
> + _handlerList.append(weakref.ref(handler, _removeHandlerRef))

Corrected in r76509. Florent, thanks for catching this (and for the patch).

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue6615>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Python bugs RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.