Merge the rewritten contributor history through ecf0027, including bounded queues and buffers, safer packet handling, audio resource cleanup, regression coverage, and the memory watcher.
Co-authored-by: Brandon McGinty <git@bmcginty.us>
Records resident memory alongside the Go heap size and dumps heap and
goroutine profiles once memory passes a threshold, while the process is
still alive to ask.
The ratio between the two numbers is what identifies the source: barnard's
Go heap sits at a few megabytes in normal operation, so memory growth with a
flat Go heap points at cgo allocations in OpenAL, opus or rnnoise, which the
Go collector cannot see and therefore never applies back pressure to.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each tick drains one fixed chunk per source, so if the encoder stalls the
loop never makes the deficit up and the backlog only grows from there. The
queues had no cap, so a long recording against a slow encoder grew for as
long as it ran.
Cap each queue and keep the newest audio; discarding the newest instead
would only push the recording further behind.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The connect channel was unbuffered. Once DialWithDialer has returned on its
synchronization timeout nothing reads that channel again, so a Reject
arriving afterwards blocked handleReject, and with it readRoutine, forever.
That leaks the goroutine along with the client, its user and channel maps
and its read buffer, and because readRoutine never reaches its exit path the
ping and UDP routines are never signalled to stop either.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The buffer grew to the largest packet ever received and was never given
back, so a single large ACL, user list or channel comment pinned its full
size, up to the ten megabyte packet limit, for the life of the connection.
Keep a modest buffer between packets and allocate larger ones only for as
long as they are needed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Buffers had a lower bound but no upper one, and it is allocated per speaking
user twice over: as a queue of decoded frames and as OpenAL playback
buffers. Each buffer holds up to one maximum sized frame, so a large value
multiplied by a populated channel is a substantial amount of memory.
Cap it, and reject the flag up front so the failure names the flag rather
than surfacing later as a dial error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every connect built a fresh stereo encoder for file playback. Each one holds
a little under a megabyte of encoder state, so on a flaky link that is close
to a megabyte of churn per reconnect for no benefit; the encoder is already
reset when file playback ends.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The saver reserves its output file exclusively, so opening it a second time
fails. connect opened a new one per connection and a disconnect closed it,
which meant the first reconnect died with "file exists" instead of resuming.
Open the saver once and re-attach it on reconnect, and close it on exit
instead. Detaching and closing are now separate, since only shutdown wants
both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
connect assigned b.Stream without releasing the stream already there, and
OnDisconnect started a reconnect loop unconditionally, so two connects could
race to install a stream. The loser was simply overwritten.
An overwritten stream is never destroyed, so it keeps its OpenAL device, its
render thread, and its entry in the shared audio listener list, which only
Destroy removes. It therefore stays subscribed for the life of the process
and every later audio packet from every user is dispatched to it as well: a
goroutine, a packet queue and a set of playback buffers per orphan, per
user. The file playback stream and the tone test saver were replaced the
same way.
Release whatever is being replaced, and allow only one reconnect loop at a
time.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Packets are removed from the jitter buffer by resliceing past them, which
leaves the popped entries in the backing array. Each one pins a decoded
audio frame until the array is next reallocated.
Clear the slot before resliceing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Destroy detached the audio listener, which closes each per-user stream
channel and ends the goroutines OnAudioStream started, and then immediately
destroyed the OpenAL context and closed the renderer without waiting for
them. Those goroutines delete their OpenAL source and buffers through
s.render, which returns false once the renderer is closed, so the deletes
were silently skipped and the resources were left to the device.
Track the goroutines and let them finish first, bounded so a wedged renderer
cannot hang a reconnect. Both device close calls now report failure as well:
alcCloseDevice frees nothing and returns false while a device still has
contexts, buffers or sources outstanding, and dropping the handle after that
leaks the device and everything it owns somewhere the Go collector cannot
see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AudioListeners.Attach set the tail pointer only when the list was empty, so
tail stayed on the first item ever attached. Once anything detached, the next
attach linked itself onto a node that was no longer in the list and that
listener never received audio again.
The equivalent code in Listeners.Attach is correct; this one had diverged
from it. The reconnect path detaches and re-attaches on every cycle, so this
was reachable in normal use.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The length is taken from the packet and only checked against the upper
bound, so a negative value passed the check and then panicked on the slice
expression that follows.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
handleChannelState applied whatever parent the server named, so a channel
could be made its own ancestor. That leaves Parent/Children cyclic, and
everything that walks the channel tree afterwards recurses until it exhausts
memory.
Ignore such a move and keep the existing parent rather than corrupting the
graph. The ancestry walk is itself bounded so a graph that is already cyclic
cannot hang the check.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rebuild_rec followed parent/child links with no depth limit, so a channel
graph in which a channel is its own ancestor recursed until the process ran
out of memory.
Cap the depth and give the whole rebuild a single node budget. Accumulating
into one slice rather than returning a new slice per node makes that budget
apply to the tree as a whole instead of to each level, and removes an
allocation per node.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AddLine re-wrapped every stored line on each append, and the buffer it
re-wrapped had no upper bound, so the work a session did grew as the square
of its length. Wrapping also built each display line by concatenating one
rune at a time, reallocating per character.
Wrap just the line being added, build it with a strings.Builder, and cap the
retained history. Trimming is done a block at a time because it forces a
rebuild; discarding a single line per append would re-wrap the whole buffer
again.
Measured over the same benchmark, 4000 lines went from 42s and 18.8GB
allocated to 4ms and 1MB, and 20000 lines now complete in 85ms.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replace the shell implementation with Python while keeping the same
dialog-based interface and the same file layout under
~/.config/barnard.
The shell version parsed servers.conf with string operations that lost
whitespace and silently discarded entries it could not read, and every
new feature meant more quoting and subshell handling.
Refuse to start on a servers.conf we cannot interpret.
Unknown keys, stray sections, and invalid ports now name the file and
line instead of being dropped and rewritten over the top of the user's
file. Duplicate server names are reported rather than silently
collapsed.
Keep a backup of the previous server list and flush it to disk.
A save is written to a temporary file, the old one is copied to .bak,
and both the file and its directory are synced before the replace.
Confirm before overwriting or removing a server.
Bracket IPv6 addresses when building the -server argument.
The shell version joined address and port with a colon, which is not a
usable address for an IPv6 literal.
Take the hostname from os.uname rather than the environment.
HOSTNAME is a shell variable and is not exported, so the connect
username lost its host part.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add UDP_ENCRYPTION.md describing the OCB2 crypt state, the 1.5 native
protobuf envelope, nonce handling, and the version negotiation that
selects between the 1.5 and legacy payload formats.
The protocol is only described across several Mumble source files, and
getting the nonce direction wrong is not visible as anything except
audio that never arrives.
Add WINDOWS.md recording what builds there today and what does not.
Update the README for the new command line options and the tone test
mode.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add integration tests behind a build tag that connect to a local
Mumble server and check the audio path end to end.
Unit tests cover the codec and the packet format separately, but
neither catches a mismatch that only appears against a real server.
These send a known tone and verify the frequency, packet ordering, and
sequence continuity that come back, at 10 ms and longer intervals, over
native UDP and over the TCP tunnel.
Add a reference-vector test for the 1.5 protobuf envelope carried
inside the TCP tunnel.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Record decoder creation, sequence numbers, frame lengths, and decode
results for each tunneled audio packet, and note when a slow listener
has a packet dropped.
Diagnosing a codec or ordering problem previously meant adding print
statements and rebuilding. These sit at info and debug, so they cost
nothing unless -logfile is given.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Route network and audio callbacks through a bounded UI queue.
Protocol handlers, the audio thread, and key handlers all drew to
termbox widgets directly, which is a data race against the render
loop. postUI is now the only path from a callback to a widget, work is
dropped during shutdown, and the queue never blocks the caller.
Guard the mutable client state with mutexes.
Connection and transmission flags, the selected user, the muted
channel set, and the audio stream pointer were each read and written
from at least two goroutines. Lookups that walk the client's user and
channel maps take the client lock and copy what they need.
Snapshot the display string when a tree item is built.
Tree items held live pointers and formatted themselves during
rendering, so a user removed between rebuild and draw was dereferenced
on the render path.
Cancel reconnect retries on shutdown and release audio before
reconnecting.
Quitting during a retry left the goroutine sleeping until its timer
expired, and a reconnect built a second OpenAL stream on top of the
first. Cleanup is idempotent so repeated disconnect events are safe.
Make UI shutdown idempotent and join the event poller.
Close could be called more than once, and the polling goroutine could
be left trying to deliver an event after Run had returned. Run now
also reports a termbox initialization failure instead of returning nil.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Append the default port only when the address does not already have
one.
The check looked for any colon, so a bare IPv6 literal such as
2001:db8::1 was treated as already carrying a port and was passed
through unusable, while a bracketed literal without a port never got
one appended.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add -tone-test to transmit a generated 440 Hz Opus tone and save
incoming audio to the file named by -tone-out.
Diagnosing an audio problem previously required a working capture and
playback device, which is exactly what is in question. This mode opens
no OpenAL device at all, so the network and codec path can be tested
on a machine with no sound hardware.
Open the capture file before starting the generator.
Starting transmission first left a tone goroutine running with nowhere
to write when the path was unusable.
Refuse file playback while in tone test mode, and ignore the
microphone volume keys.
Both operate on a stream that does not exist here.
Stop the generator and detach the file saver on disconnect.
A reconnect otherwise attached a second saver to the same file.
Add -auto-transmit to key the microphone as soon as the connection is
up.
Useful for a bot or a monitoring client that should never need a
keypress. It runs from both the connect event and the point where the
audio stream is created, because the server's welcome arrives before
the stream exists.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Remove control, DEL, and bidi characters before display.
HTML escaping was the only filter, which leaves ANSI and OSC escape
sequences intact. A remote user could move the cursor, recolour the
screen, or reorder what was shown by putting escape codes in a
message, a nickname, or a channel name. Names rendered in the channel
tree go through the same filter now.
Keep the text box cursor and the prompt on rune boundaries.
Both indexed by byte, so editing a line containing multi-byte
characters could split one and render replacement characters.
Handle text view lines that carry no timestamp.
Toggling timestamps assumed every stored line had one and sliced past
the end of lines that did not.
Ignore key events on an empty tree.
The handlers indexed the item list before checking that it had any
items.
Remove the beep helpers.
They shelled out to an optional "beep" binary and panicked when it was
absent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Give the notification queue a buffer and drop events when it is full.
The channel was unbuffered, so a slow or hung notification command
blocked whichever UI or network callback happened to raise the event.
Expand the command placeholders in a single pass.
Substituting %event, then %who, then %what meant text arriving in an
earlier field could contain a later placeholder and have it expanded,
letting a remote user inject their own text into the command.
Move the command runner behind a build tag and drop the POSIX default
on Windows.
The helper script it pointed at does not exist there, so the default
was a command that could only fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reject a FIFO path that points to an existing non-FIFO object.
Previously we removed whatever was at that path before creating the
pipe, so pointing -fifo at a regular file silently deleted it. An
existing FIFO is reused and only a missing path is created.
Move the implementation into fifo_unix.go and fifo_windows.go.
This lets the Unix build use syscall.Mkfifo directly, which is not
available on Windows.
Prevent the reader from endlessly spinning on a closed FIFO after an
error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Kill ffmpeg's whole process group rather than just the process.
ffmpeg spawns helpers for some inputs, so stopping playback left them
running and holding the output pipe open. The player now starts the
command in its own process group and signals the group.
Wait for the playback worker to finish before starting a new file.
Playing a second file while the first was shutting down left two
workers writing to the same stream.
Make pausing nonblocking.
The pause path could block on a full pipe and hang the UI thread that
requested it.
Install and reset the stereo encoder under the client lock.
File playback swapped the encoder field directly while a voice frame
could be encoding with it, and a finished file left the encoder
carrying state into the next one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Apply the persisted microphone volume when the stream is created.
The value was written to the configuration on every adjustment but
never read back, so the microphone returned to full gain on each
start.
Read a saved volume of zero as zero rather than as a missing value.
A user who muted their microphone and quit came back unmuted.
Save the configuration after a volume change and report a failure.
The setter only updated the in-memory value, so the new level was lost
unless something else happened to save afterwards.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Lower the gain ceiling and slow the attack of the automatic gain
control.
It amplified room noise between words hard enough to be audible as a
rising hiss whenever the speaker paused.
Add an AGC toggle on F12 and an /agc command.
AGC has always been applied unconditionally, which is wrong for a
microphone that is already levelled by hardware or by the system
mixer. The preference is saved and reapplied on connect, defaulting to
on so existing setups are unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Return errors from the save path rather than panicking.
Every write panicked on failure, so a read-only or missing
configuration directory killed a running client. The parent directory
is created when absent, and callers now show the failure in the output
window and carry on.
Write through an unpredictable temporary file.
The old fixed ".tmp" name next to the configuration was a symlink
target an attacker could plant in advance.
Serialize configuration reads and writes.
Hotkey handlers, the audio thread, and the connection callbacks all
touch the same structure, so saves could interleave with updates.
Parse addresses with SplitHostPort.
Splitting on every colon broke IPv6 addresses and panicked outright on
an address with no port. Both now fall back to Mumble's default port.
Fail immediately when an explicitly requested config file is missing
or is not a regular file.
Silently falling back to defaults hid a mistyped -config path.
Supply defaults for the clear-output and scroll-to-top and -bottom
hotkeys.
The UI registered listeners for them but the configuration never
filled the keys in, so the bindings were nil and the keys did nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add -audio-interval to choose a 10, 20, 40, or 60 ms packet duration.
Longer packets cut per-packet overhead on slow or lossy links at the
cost of latency. Anything else is rejected at startup rather than
silently truncated to 10 ms frames.
Step the audio sequence by the frame duration.
Sequence numbers are Mumble timestamps in 10 ms units, so a 60 ms
packet advances the counter by six. Incrementing by one made the
receiver see every packet as arriving far too early.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
