853944 - Datachannel not deleting critical sections on Windows.

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Ethan Hugg [:ehugg]]

if WINVER < 600 then the critsec and events created in sctpuserspace.c::InitializeXPConditionVariable are never destructed.

Caught by AppVerif because struct socket* mMasterSocket was being freed with un-deleted locks inside.

Comment 1

[Ethan Hugg [:ehugg]]

Attachment: Datachannel - delete CriticalSection and Events from conditional variables on Windows

Comment 2

[Ethan Hugg [:ehugg]]

Comment on attachment 728387 [details] [diff] [review]
Datachannel - delete CriticalSection and Events from conditional variables on Windows

Review of attachment 728387 [details] [diff] [review]:
-----------------------------------------------------------------

Jesup, not sure who should review, so I'm putting you down for now.

Comment 3

[Ethan Hugg [:ehugg]]

Try push in the naive hope that it might fix the oranges from Bug 837035

https://tbpl.mozilla.org/?tree=Try&rev=632de62d79e6

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 728387 [details] [diff] [review]
Datachannel - delete CriticalSection and Events from conditional variables on Windows

review to upstream source (I can't review my own patch... ;-)

Comment 5

[Michael Tüxen]

Attachment: Updated patch

I have modified the patch to move the version dependent stuff to
sctp_os_userspace.h.

This patch has been committed upstream as
http://code.google.com/p/sctp-refimpl/source/detail?r=8440

Thanks for the report!

Best regards
Michael

Comment 6

[Michael Tüxen]

Comment on attachment 728387 [details] [diff] [review]
Datachannel - delete CriticalSection and Events from conditional variables on Windows

Review of attachment 728387 [details] [diff] [review]:
-----------------------------------------------------------------

Please see a minor changed in the attached "updated patch". This has been committed upstream.

Comment 7

[Ethan Hugg [:ehugg]]

Unsurprisingly, this does not fix the problem with pushing Bug 837035 so I'm not going to link the bugs, nor is it urgent enough to push before we get it from upstream imho.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 728387 [details] [diff] [review]
Datachannel - delete CriticalSection and Events from conditional variables on Windows

Clearing review as this is fixed upstream

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Fixed by checkin for bug 855620