#twisted-dev

Tickets pending review: https://tm.tl/#8894, #8900, #6800, #8922 (glyph), #8325 (the0id), #8920

huh TIL OpenSSL only supports negotiating a contiguous range of protocol versions

you can't say "TLS 1.0 or TLS 1.2 but not TLS 1.1"

"In clients, when a protocol version is disabled without disabling all previous protocol versions, the effect is to also disable all subsequent protocol versions."

haha

well that's interesting

idnar: hey so

idnar: spoiler alert: openssl is bad

specifically, you can adopt this general principle when interacting with it: if its interface *implies* something about its underlying operation, that probably has no bearing on its actual underlying operation

glyph: don't worry I already saw that episode

glyph: can't wait until reimplement ssl in Python

moshez: ashfall's working on it

as is runciter

glyph: I know :)

moshez: hey do you want to fix some memory leaks

in general, yes

specifically, I have an ingress op I need to leave for in 3

moshez: see the mailing list when you have some time :)

Tickets pending review: https://tm.tl/#8894, #8900, #6800, #8922 (glyph), #8325 (the0id), #8920

runciter: welcome back!

hello

how are you today

runciter: leaking memory :(

i saw the email!

when it rains it pours

runciter: yeah.

runciter: kinda bummed about that one especially

glyph: i think there *is* an issue with TLSMemoryBIOProtocol, btw

well, i don't know yet, but i think so

glyph: is this all happening because of discussions about causality loggers

runciter: TLSMemoryBIOProtocol not coping with heartbeats properly you mean?

glyph: specifically if there's a heartbeat that also data

glyph: evidently TLS can stick application data into a heartbeat

glyph: that data never makes it out of the BIO

glyph: (on the server side)

runciter: wt fffffffff

it seems like this is an issue with OpenSSL.Connection

runciter: yeah that is just straight up OpenSSL's fault.

runciter: we're not fixing that one :)

glyph: now it's also possible that the C code is completely wrong

runciter: a mistake, in C code? heavens to betsy!

glyph: yeah, i mean, all twisted's doing is trying to read out of the dang old BIO and getting a WantRead

glyph: i know right

so of the billion things to do i'm gonna deprioritize this a little

(also who even uses heartbeats)

i thought everyone disabled it after heartbleed

because baby, bathwater, etc

you don't _need_ heartbeats ove TCP

look it's not a particularly good baby but

it's a DTLS feature

has anyone ever used DTLS

hawkowl: it is disabled in recent openssls

hawkowl: i think uh some things?

runciter: woah, they _completely_ got rid of dtls?

runciter: anyway did you see on the ML

glyph: http://stackoverflow.com/questions/40604545/twi... ?

i'm looking into pympler

glyph: i don't think they turned off DTLS, but they did turn off heartbeat

runciter: that's the one

glyph: I think that leak is due to LogBeginner._initialBuffer.

tomprince: ZOMG

tomprince: you are a genius

tomprince: did you use a tool to arrive at that answer?

I used a combination of heappy and gc.get_objects/get_referrers and instrumenting the example.

Although I first got distracted by the reactor collecting up to 50 cancelled DelayedCalls.

Once I reconfirmed that it was Logger instances, I looked at the namespaces of uncollected Loggers, which where ~all twisted.internet.endpoints.OneShotFactory

Then looking at the referrers of those instances, it all looked like log message dicts.

Then I tried starting to log to /dev/null, and the memory flat-lined.

sorry, back

tomprince: you are fast :)

is this because people arent starting their loggers?

glyph: Is log_logger actually valuable information? It seems like it is redundent with log_source and log_namespace.

there was a ML post about some diagnosing a memory leak but it was really because they never actually started their loggers :D

the example isn't leaking memory for me?

or does it take a while?

I needed to change the example so it connected to itself, and also make an initial connection.

i've got a twisted web server running on 8080 and netcat running in a loop

hm

isnt the initial logger supposed to keep only a bounded amount of messages :o

heh

tomprince: sorry to be a pain, but can you share the version that manifests the leak?

runciter: Well, it isn't an unbounded leak (as asdf said, the initial logger is bounded), but it does use ~double the amount of memory it would otherwise use.

65536 event dictionaries, right?

(the deque's maxlen is that)

Yeah.

runciter: http://paste.fedoraproject.org/498185/38309148/ (which has a ton of commented out debugging code)

tomprince: ty :)

i'm reproducing it with my own dumb hacked one now

It uses ~535M when collecting logs, ~232M when discarding logs (with beginLoggingTo([])).

well this is interesting

i can't reproduce it when using muppy's tracker

maybe its growth rate slows so much i don't notice the increase

runciter: guppy's? It does seem to slow things down enough that it isn't as visible.

tomprince: oh i was hoping to using this snazzy thing: https://pythonhosted.org/Pympler/#usage-examples

(second example)

watching the memory usage in top shows that it stays relatively constant between connections

yeah

but it also doesn't show a meaningful diff, which is probably just user error

tomprince: starting logging killed the leak for you?

Yeah.

Well, it was a bounded leak anyway (~535M vs ~232M steady state)

tomprince: Well, this is reassuring. connectProtocol doesn't leak any more or less than using the endpoint directly?

I didn't try with the endpoint directly. Let me se.

glyph: One difference is that you can share a factory if you use an endpoint directly (which with the recursive calling my modified example has) so there is only a single starting message, rather than a fresh one for every connection.

glyph: Adding noisy=False to https://github.com/twisted/twisted/blob/trunk/s... also gets rid of the leak.

yeah so the leak is not starting your logger

(start your logger!!!!)

maybe i should write a blog post about twisted's logging systems

but i also should sleep

zzz

Tickets pending review: https://tm.tl/#8894, #8900, #6800, #8922 (glyph), #8325 (the0id), #8920

Tickets pending review: https://tm.tl/#8894, #8900, #6800, #8922 (glyph), #8325 (the0id), #8920

new web defect https://tm.tl/#8935 by exarkun: _GenericHTTPChannelProtocol.dataReceived ValueError has spelling error