Skip to content

Fix member function call on null-pointer instance of SignalingClient#940

Merged
pjungkamp merged 2 commits into
masterfrom
fixes-webrtc
Jul 8, 2025
Merged

Fix member function call on null-pointer instance of SignalingClient#940
pjungkamp merged 2 commits into
masterfrom
fixes-webrtc

Conversation

@pjungkamp
Copy link
Copy Markdown
Contributor

Bug

libwebsockets calls the protocol callbacks for our webrtc signaling client even when the connection is not explicitly initiated from the signaling client code. This causes the protocolCallbackStatic function to cast a null-pointer to a SignalingClient and call the non-static protocolCallback.

Fix

I moved the entire protocolCallback logic into the static member function and removed the non-static member function.

Context

I found this bug using -fsanitize=address and -fsanitize=undefined. This bug occurs on an idle villas-node process, without any configuration or user input.

$ ./build/src/villas-node
11:10:04 info node             This is VILLASnode v0.11.0-db2bd63-debug (built on Jan  1 1980, 00:00:00)
11:10:04 info signals          Initialize subsystem
11:10:04 warn node             No configuration file specified. Starting unconfigured. Use the API to configure this instance.
11:10:04 info memory           Initialize memory sub-system: #hugepages=100
11:10:04 warn memory:mmap      Running in an unprivileged environment. Hugepages are not used!
11:10:04 warn memory           Running in an unprivileged environment. Memory is not locked to RAM!
11:10:04 info kernel:rt        Initialize sub-system
11:10:04 warn kernel:rt        We recommend to use an PREEMPT_RT patched kernel!
11:10:04 warn kernel:rt        You might want to use the 'priority' setting to increase VILLASnode's process priority
11:10:04 warn kernel:rt        You might want to use the 'affinity' setting to pin VILLASnode to dedicate CPU cores
11:10:04 info api              Starting sub-system
11:10:04 info web              Starting sub-system
11:10:04 info api              Started worker
11:10:05 info web              Started worker
11:10:05 info stats             Node      │      Recv │      Sent │      Drop │      Skip │  OWD last │  OWD mean │ Rate last │ Rate mean │  Age mean │   Age Max │ Signals
11:10:05 info stats                       │      pkts │      pkts │      pkts │      pkts │      secs │      secs │   pkt/sec │   pkt/sec │      secs │       sec │ signals
11:10:05 info stats            ───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼─────────
/home/pjungkamp/Projects/VILLASframework/node/lib/nodes/webrtc/signaling_client.cpp:107:29: runtime error: member call on null pointer of type 'struct SignalingClient'
^C11:10:35 info node             Received Interrupt signal. Terminating...
11:10:35 info api              Stopping sub-system
11:10:35 info api              Stopped worker
11:10:35 info web              Stopping sub-system
11:10:35 info web              Stopped worker
11:10:35 info node             Goodbye!

@pjungkamp pjungkamp requested review from n-eiling and stv0g as code owners July 8, 2025 12:14
Signed-off-by: Philipp Jungkamp <philipp.jungkamp@rwth-aachen.de>
Comment thread lib/nodes/webrtc/signaling_client.cpp
Comment thread lib/web.cpp
@n-eiling n-eiling requested a review from Copilot July 8, 2025 12:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the WebRTC signaling client’s protocol callback to eliminate a crash when the callback is invoked on a null instance. It inlines the former non-static protocolCallback into the static entry point and updates registration in lib/web.cpp.

  • Switch .callback in lib/web.cpp to the unified static protocolCallback
  • Merge logic into the static callback and remove the old non-static overload
  • Adjust internal calls to use the session user pointer (c->…)
  • Add gdb to development tools in flake.nix

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
lib/web.cpp Updated protocol registration to use the new static callback
lib/nodes/webrtc/signaling_client.cpp Inlined non-static logic into protocolCallback, removed old overload, fixed dummy callback invocation
include/villas/nodes/webrtc/signaling_client.hpp Removed obsolete member declaration, updated static callback signature
flake.nix Added gdb to development dependencies

Comment thread lib/nodes/webrtc/signaling_client.cpp
@pjungkamp pjungkamp requested a review from n-eiling July 8, 2025 13:37
This fixes a bug where libwebsockets would call the protocol
handler with a NULL `user` pointer on initialization. This caused the
`SignalingClient::protocolCallback` non-static function to be invoked on a
null pointer instance.

Signed-off-by: Philipp Jungkamp <philipp.jungkamp@rwth-aachen.de>
@pjungkamp pjungkamp merged commit 2c3d4e7 into master Jul 8, 2025
3 checks passed
@pjungkamp pjungkamp deleted the fixes-webrtc branch July 8, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants