Version 0.1.9.1

Version 0.1.9

Only change the receiving nicesock for udp-turn for ice-tcp

The code was in the wrong spot, for OC2007, we need to fix the
nicesock used only in the case of udp-turn-over-tcp for OC2007
compatibility mode. With the current code, it was thinking that
the data came from TURN for every udp packet.
This should hopefully fix : https://github.com/EricssonResearch/openwebrtc/issues/85

docs: Enable documentation checks during `make check` by default

Now that they pass, we should endeavour to ensure they continue to pass.

docs: Add 0.1.8 API to the documentation index

This should have been included before. This fixes `make check` in docs/.

docs: Add various missing documentation comments and update sections.txt

This straightens out the last few bits of the documentation and almost
allows `make check` to pass in docs/.

docs: Inform gtk-doc of the libnice namespace

This allows it to order the index a little better.

docs: Use gtk-doc no-tmpl mode

Templated mode has been deprecated for a long time and will be removed
in future versions of gtk-doc. Switch to no-tmpl mode for faster
documentation builds and future-proofness.

This does not bump our gtk-doc dependency, since gtk-doc 1.9 supported
no-tmpl mode.

docs: Switch from gtk-doc SGML mode to XML mode

We already used XML everywhere; just formally switch to XML mode and
eliminate the last few traces of SGML.

agent: Document G_MESSAGES_DEBUG and NICE_DEBUG

As requested on the mailing list, expand the documentation for
nice_debug_enable() to mention them.

agent: Handle EWOULDBLOCK when transmitting pseudo-TCP segments

The pseudo-TCP code previously didn’t handle EAGAIN or EWOULDBLOCK
errors from the low-level NiceSocket code. This caused pseudo-TCP
connections to be dropped if the transmitting socket ever filled up,
which could cause problems on high bandwidth connections.

Fix the issue by effectively dropping the packet on EWOULDBLOCK. This
will eventually get picked up by the pseudo-TCP recovery mechanism,
retransmitting the packet and throttling the sender. This should
hopefully reduce the system resource usage which caused EWOULDBLOCK in
the first place.

Spotted and debugged by Radosław Kołodziejczyk
<radek.kolodziejczyk@gmail.com>.

https://bugs.freedesktop.org/show_bug.cgi?id=87344

pseudotcp: Do roll-over safe assertion

In case of a roll-over, only the difference matters.

agent: Add debug output for lifetime of Components and Streams

tests: Use G_SOURCE_[CONTINUE|REMOVE] instead of booleans

Makes the test code a little clearer. Introduces no functional changes.

tests: Fix a memory leak in test-send-recv

If n_messages ≠ n_valid_messages.

Add ability to build static GST plugins

Note that this only works for 1.0 plugin.

https://bugs.freedesktop.org/show_bug.cgi?id=85929

agent: Expand simple example in documentation to mention GMainContext

wrt the ML thread:
http://lists.freedesktop.org/archives/nice/2014-October/000981.html

agent: Close pseudo-TCP socket earlier in component_close()

This tries to mitigate the race condition between finishing the TCP FIN
handshake and closing the underlying sockets, but it’s impossible to
mitigate properly without API changes. See the comment.

stun: Rename a symbol to avoid a naming clash with OpenSSL

The symbol is not exported, but nevertheless leaks in the static version
of libnice, due to limitations on symbol visibilities with static
libraries. OpenSSL has a symbol named RAND_bytes(), which ours clashes
with if the two are linked together statically.

Avoid this by prefixing ours with ‘nice_’.

Fix documentation relating to how remote crendentials should be set for new ICE format

agent: ignore externally set peer-reflexive candidates

All discovered peer-reflexive candidates should be added internally in
conncheck.c and should have the sockptr set. We ignore any prflx
candidates added by an external application because their NULL sockptr
could cause a crash in conn_check_send().

Version 0.1.8.1

Version 0.1.8

agent: Remove extra post condition

It prevent the GError from happening

udp-turn: Don't dereference priv before check that it's valid

agent: Pass all packets that are not acceptable STUN packets to the application

We attempted to not pass through some packets that looked like
valid STUN but were ot acceptable to us, but that dropped some application
packets.

agent: Re-emit gathering done after a new relay has been set

If the stream was ever gathering and a new relay has been set,
then re-emit the candidate=gathering-done signal

pseudotcp: Print unsigned int with %u not %d

agent: Add function to retrieve the current component state

agent: Make the stun-pacing-timer app configurable

There is no reason to only allow modifying it in the new().

pseudotcp: Set default RTO to 1 second

The newer RFC 6298 recommends 1 second instead of 3.

conncheck: Computer STUN retransmission timeout (RTO) dynamically

This is how it is specified in RFC 5245 section 16

discovery: Remove extra \n in g_debug()

agent: Only add pairs once on gathering done

Also don't try to re-add pairs that already have been added

conncheck: Insert the candidates sorted after setting the priority

conncheck: Insert peer reflexive pair sorted

The rest of the list is sorted, but this was just appended, making it likely
to be dropped if the list grows too long.

conncheck: Don't remove elements in the conncheck list while iterating it

priv_limit_conn_check_list_size() would remove elemtns from the conncheck_list
while the calling functions were iterating it. Now instead just mark them as
cancelled. Then later, at the outer function, free all cancelled elements to
prevent the list from growing out of bounds.

agent: Avoid leaking UPnP mappings between streams

Clean up the UPnP mappings of each stream when closing it

agent: Avoid restarting the GUPnP client on every gather

This would cause mappings to be dropped on every new gather, which is bad!
Instead, keep the same one with the mappings, and just drop the timer to ignore
new discovered mappings afterwards.

agent: Ignore UPnP mappings after the UPnP discovery has finished

component: On ICE restart reset selected pair priority to 0

This is to ensure that the result of the new negotiation will always
have priority.

agent: Add some preconditions to public API

To try and avoid NULL NiceCandidates entering our internal state.

build: Update .gitignore

outputstream: Fix double unref

agent: Remove socket source on HUP

agent: Declare the stream to be CONNECTED only if a pair is selected

Don't change the state if no pair is selected. Otherwise we get
a component that claims to be CONNECTED but has no selected pair.
Also, set the selected pair before announcing the state change.

nice.pc: Include ${includedir} directly

It doesn't make sense to tell applications to include <agent.h>,
including <nice/agent.h> is much safer. So tell the applications
to use the includedir directly.

agent: Document interaction between closing and removing streams

GIOStream and nice_agent_remove_stream() interact slightly subtly when
closing down the stream.

See: https://bugzilla.gnome.org/show_bug.cgi?id=735754

tests: Add more helgrind suppressions

agent: Delay some assignments until the agent is locked

This makes helgrind slightly happier.

agent: Warn if a Component is freed without first being closed

agent: Close Streams before freeing them

This could result in leaking the stream’s un-closed state otherwise, in
any case where a NiceAgent is finalised without
nice_agent_remove_stream() being called on all its streams.

tests: Add a Valgrind suppression file

To be used with `make check-valgrind`.

tests: Fix various memory leaks in the unit tests

The theory being that if we can get check-valgrind to pass, we could
acutally use it to find more memory leaks and other errors in the agent.

agent: Check STUN buffer is non-NULL before getting its message ID

This should fix a crash, as detected by Valgrind:
  ==28354== Invalid read of size 2
  ==28354==    at 0x4C2B5B0: memcpy@@GLIBC_2.14 (in
    /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==28354==    by 0x50C17E2: stun_message_id (stunmessage.c:658)
  ==28354==    by 0x509E4E7: candidate_check_pair_fail (conncheck.c:254)
  ==28354==    by 0x50A4EDB: conn_check_prune_socket (conncheck.c:3145)
  ==28354==    by 0x509B6F8: component_io_cb (agent.c:3951)

agent: Fix a leak of a GCancellable and its GSource

The GSource holds a reference to the GCancellable, so needs to be
explicitly removed from the GMainContext when it’s finised with.

agent: Fix a minor leak in an error handling path

tests: Move a closure from the heap to the stack

There is no need for this to be heap-allocated.

conncheck: Don't use CandidateRefresh after freeing it

socket: Fix a leak on the slow TURN packet handling path

agent: Eliminate a memset() for local variable initialisation

Stack variables can be initialised to zero by explicit assignment.

This introduces no functional changes.

agent: Use g_slist_free_full() to reduce code

This introduces no functional changes.

agent: Add some preconditions to internal API

To try and avoid NULL NiceCandidates entering our internal state.

agent: Add some preconditions to public API

To try and avoid NULL NiceCandidates entering our internal state.

agent: Clear GSource timeout also for GUPnP timeout

Fix regressions introduced by the following patch:
Clear existing GSource timeouts before adding new ones

agent: Drop valid but unmatched STUN packets

Rather than passing them through to the application. Invalid STUN
packets are still passed through, but it causes unnecessary noise and
corruption for higher-level applications to receive STUN packets they
weren’t expecting.

This is permitted by RFC 5389, §7.3.0:
    If any errors are detected, the message is silently discarded.
    In the case when STUN is being multiplexed with another protocol,
    an error may indicate that this is not really a STUN message; in
    this case, the agent should try to parse the message as a different
    protocol.
where I interpret ‘error’ to mean ‘validation error’ rather than (e.g.)
‘unrecognised attribute’ or ‘unmatched response’ where the STUN packet
is otherwise perfectly formed.

agent: Remove dangling pointers on NiceSocket destruction

If a NiceSocket is destroyed, various pointers are currently left
dangling to it in the conncheck state. These can cause crashes if (for
example) a CandidateCheckPair with such a dangling pointer is then used;
the GSocket methods will fail.

Fix this by explicitly removing the socket and all NiceCandidates which
wrap it from various areas of the state.

agent: Factor out state transition to FAILED for CandidateCheckPairs

This introduces no functional changes.

agent: Factor out free function for IncomingCheck

agent: Improve comments for container element types

To allow enhanced grepping for what structs point to other structs.

agent: Clear existing GSource timeouts before adding new ones

Modify agent_timeout_add_with_context() to force destroying and freeing
of an existing GSource before overwriting it with a new one (probably
with an updated timeout period).

This fixes a case in priv_map_reply_to_relay_refresh() where the TURN
candidate refresh timer was being overwritten with a new one, without
the old one being destroyed. This lead to two timeouts existing, only
one of which would be destroyed when the CandidateRefresh struct was
freed, leaking the other one (in the main context) and allowing it to be
later dispatched with a dangling CandidateRefresh pointer.

The modification to agent_timeout_add_with_context() should prevent this
happening in new code in future.

agent: Add names to timer GSources

Modify the agent_timeout_add_with_context() utility function to
automatically add names to the timer GSources it creates. This makes
them a little easier to identify when debugging.

socket: Return early from socket functions if the socket is closed

Explicitly check whether the socket is closed (universally represented
as sock->priv == NULL) before doing anything else in the socket methods.
This should safely return from unusual situations where the socket has
been closed and part-destroyed but still ends up having send() or recv()
called on it.

socket: Fix return values of socket_send_messages in socks5.c

The return value is the number of messages sent, or -1 on error — not a
boolean.

stun: Make a warning message more prominent

Since dropping a STUN message due to having insufficient buffer space in
libstun can cause ICE negotiation to fail. So we want it to be more
obvious in the logs.

agent: Put one StunAgent per Component

Otherwise we risk running out of space in the space limited saved ids
list. So the easiest way to do that is to put one StunAgent in each
component. It may be advisable to just give up on not allocating
memory inside libstun and just use a regular hash table in there
instead.

conncheck: Fetch stream once instead of twice

agent: Agent can reach EOS after having received something

The agent can reach EOS after performing one or more non-empty reads in
its read loop, and then hitting EOS on the underlying stream. That means
reached_eos is TRUE, but n_valid_messages is non-zero.

Weaken the postcondition to reflect this.

agent: Fix locking when calling component_free()

As mentioned in the previous commit message, component_free() requires
the agent lock to be released, but component_close() requires it to be
held. Fix the locking in nice_agent_remove_stream() to do that.

agent: Split component_free() into component_[close|free]()

Closing a component and freeing it are slightly conceptually different.
As it happens, freeing it can result in the disposal of the component's
NiceIOStream, which requires the agent lock to be released — but closing
the rest of the Component requires the agent lock to be held. Splitting
the function up simplifies locking handling in other parts of the code
(see next commit).

agent: Fix initialisation of the agent mutex for old GLib versions

The mutex is now non-recursive. This is left over from commit 1deee693.

tests: Fix test-build-io-stream for EOS changes

From commit 7b6935c66738d855c84fba291d47ece6ce1c43e2.

agent: Add missing bracket

Bad merge in commit 67179ffd8fb1a39936c54ce611e8deeec8705ac3.

agent: Add missing stream-closed checks for pseudo-TCP

This got lost in rebasing the FIN–ACK work. component->tcp is now only
NULL before the pseudo-TCP stream is initially created — afterwards, it
is always non-NULL, but pseudo_tcp_stream_is_closed() returns TRUE if
the stream has closed (gracefully or on errors). This allows
differentiation between the states: TCP support was never initialised;
and TCP support was initialised but is now closed.

agent: Return 0 on EOS from nice_input_stream_read()

We should return 0 instead of G_IO_ERROR_CLOSED for consistency with
recv() and read().

pseudotcp: Ensure shutdown member is not overwritten

Add a few safeguards to ensure that once priv->shutdown is set, it is
not overwritten with a different value.

pseudotcp: Fix EOS return from recv() in non-FIN–ACK mode

Change pseudo_tcp_socket_recv() to return 0 if in non-FIN–ACK mode and
the socket has been shut down. This makes the behaviour of FIN–ACK and
non-FIN–ACK modes consistent.

This is a behaviour change from before the introduction of FIN–ACK mode
— previously, pseudo_tcp_socket_recv() would return -1 and set the
ENOTCONN error if called after the socket had been shut down (i.e. after
pseudo_tcp_socket_get_next_clock() had returned FALSE). The new
behaviour will hopefully not break anything.

pseudotcp: Ensure socket is closed after shutdown in non-FIN–ACK mode

If the socket is closed (with pseudo_tcp_socket_close()) with FIN–ACK
unsupported, it should transition to the CLOSED state when
pseudo_tcp_socket_get_next_clock() returns FALSE (as that’s the
old-style indication that the socket has closed).

This is a behaviour change from before the introduction of FIN–ACK:
previously, the socket would stay in the ESTABLISHED state. However,
this should not have been easily detectable by consumers of the API,
as pseudo_tcp_socket_is_closed() did not exist then.

pseudotcp: Add a PseudoTcpSocket:support-fin-ack property

This allows FIN–ACK support to be disabled entirely. This is mostly for
testing purposes, since TCP_OPT_FIN_ACK is negotiated when establishing
the connection, and is disabled if the other side doesn’t support it.

This includes an interoperability test.

tests: Fix race conditions in test-thread

As found by tsan.

agent: Close pseudo-TCP streams when closing G[IO|Input|Output]Streams

This means that users of the Nice[Input|Output|IO]Stream API can easily
close TCP connections without having to hack around with libnice
internals.

docs: Ignore some undocumentable/non-public API

pseudotcp: Add pseudo_tcp_socket_shutdown() support

This is analogous to the UNIX shutdown() function, allowing either or
both sides of a pseudo-TCP connection to be shut down.

pseudotcp: Shorten the TIME-WAIT state timeout dramatically

The TIME-WAIT timeout is typically 2×MSL (on the order of 60 seconds),
which is needed to be able to reject delayed segments from closed
conversations. However, the underlying socket layer for pseudo-TCP takes
care of channel numbering so that segments don’t end up being sent to
closed conversations. Therefore, the TIME-WAIT state can be eliminated
(by shortening the timeout). The code for the state is kept around so
that the pseudo-TCP implementation matches the TCP specification
correctly, which will simplify maintenance.

tests: Add a new segment-by-segment test framework for pseudo-TCP

This explicitly removes all timers and main loop considerations from the
pseudo-TCP testing, which considerably simplifies arranging packet swaps
and mistimings for testing purposes.

This test suite includes a few tests for the FIN–ACK support.

pseudotcp: Add optional FIN–ACK and RST support

In order to detect cases where the peer closes its connection without an
explicit in-band close message (e.g. in protocols such as Telnet where
there is none), pseudo-TCP needs to grow support for a shutdown
handshake, following the TCP FIN–ACK specification. Arguably it should
have had this all along, but Jingle apparently doesn’t need it.

This adds support for FIN–ACK to the pseudo-TCP implementation. It is
backwards-compatible, only being used if the TCP_OPT_FIN_ACK option is
specified in the SYN segment.

If enabled, full-duplex closes are supported, and the standard method
for notifying a peer of the other end closing its connection (returning
0 from recv()) is used.

Also allow rapidly tearing down a connection, discarding unsent and
unreceived data, by sending an RST segment. This preserves the ability to
do a forced socket closure with pseudo_tcp_socket_close(sock, TRUE).

It also permits graceful socket shutdown in the case where the final ACK
is lost, and one peer gets stuck in the LAST-ACK state: that peer will
eventually re-transmit its FIN segment. The other peer, in the CLOSED
state, will respond with a RST segment, and the first peer will then
reach CLOSED.

References (most useful first):
 • http://tools.ietf.org/html/rfc793#section-3.5
 • http://tools.ietf.org/html/rfc1122#page-87
 • http://vincent.bernat.im/en/blog/2014-tcp-time-wait-state-linux.html
 • http://tools.ietf.org/html/rfc675
Diagram:
 •
http://en.wikipedia.org/wiki/Transmission_Control_Protocol#mediaviewer/File:TCP_CLOSE.svg

agent: Handle EPIPE from pseudo_tcp_socket_send()

As with send(), pseudo_tcp_socket_send() may return EPIPE if the local
side of the connection has been closed using close() or shutdown(). It
currently doesn’t, but will do once pseudo-TCP FIN–ACK support has been
implemented.

agent: Don’t clear the PseudoTcpSocket on error

Instead, keep the closed object around. This allows differentiation
between the states: TCP support was never initialised; and TCP support
was initialised but is now closed.

pseudotcp: Fix a typo in some documentation

pseudotcp: Factor out state change functions for ESTABLISHED and CLOSED

Both state changes need some follow-up code executed immediately after,
which was duplicated in a number of places. Factor that out. This
introduces no behavioural changes.