- 26 Sep, 2014 3 commits
-
-
Philip Withnall authored
GIOStream and nice_agent_remove_stream() interact slightly subtly when closing down the stream. See: https://bugzilla.gnome.org/show_bug.cgi?id=735754
-
Philip Withnall authored
-
Philip Withnall authored
This makes helgrind slightly happier.
-
- 25 Sep, 2014 8 commits
-
-
Philip Withnall authored
-
Philip Withnall authored
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.
-
Philip Withnall authored
To be used with `make check-valgrind`.
-
Philip Withnall authored
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.
-
Philip Withnall authored
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)
-
Philip Withnall authored
The GSource holds a reference to the GCancellable, so needs to be explicitly removed from the GMainContext when it’s finised with.
-
Philip Withnall authored
-
Philip Withnall authored
There is no need for this to be heap-allocated.
-
- 24 Sep, 2014 1 commit
-
-
Olivier Crête authored
-
- 23 Sep, 2014 5 commits
-
-
Philip Withnall authored
-
Philip Withnall authored
Stack variables can be initialised to zero by explicit assignment. This introduces no functional changes.
-
Philip Withnall authored
This introduces no functional changes.
-
Philip Withnall authored
To try and avoid NULL NiceCandidates entering our internal state.
-
Philip Withnall authored
To try and avoid NULL NiceCandidates entering our internal state.
-
- 22 Sep, 2014 6 commits
-
-
Olivier Crête authored
Fix regressions introduced by the following patch: Clear existing GSource timeouts before adding new ones
-
Philip Withnall authored
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.
-
Philip Withnall authored
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.
-
Philip Withnall authored
This introduces no functional changes.
-
Philip Withnall authored
-
Philip Withnall authored
To allow enhanced grepping for what structs point to other structs.
-
- 19 Sep, 2014 2 commits
-
-
Philip Withnall authored
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.
-
Philip Withnall authored
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.
-
- 02 Sep, 2014 2 commits
-
-
Philip Withnall authored
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.
-
Philip Withnall authored
The return value is the number of messages sent, or -1 on error — not a boolean.
-
- 01 Sep, 2014 4 commits
-
-
Philip Withnall authored
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.
-
Olivier Crête authored
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.
-
Olivier Crête authored
-
Philip Withnall authored
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.
-
- 27 Aug, 2014 2 commits
-
-
Philip Withnall authored
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.
-
Philip Withnall authored
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).
-
- 25 Aug, 2014 3 commits
-
-
Philip Withnall authored
The mutex is now non-recursive. This is left over from commit 1deee693.
-
Philip Withnall authored
From commit 7b6935c6.
-
Philip Withnall authored
Bad merge in commit 67179ffd.
-
- 23 Aug, 2014 2 commits
-
-
Philip Withnall authored
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.
-
Philip Withnall authored
We should return 0 instead of G_IO_ERROR_CLOSED for consistency with recv() and read().
-
- 21 Aug, 2014 2 commits
-
-
Philip Withnall authored
Add a few safeguards to ensure that once priv->shutdown is set, it is not overwritten with a different value.
-
Philip Withnall authored
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.
-