Skip to content

Commit 7f12fb0

Browse files
author
Kevin Vu te Laar
committed
python-binding unit test split, socket fixes
Unit test portion split. Previously commented-out tests are now active. Fixes to the socket node resolved previous issues during testing. These arised when C-API functions were called through the bindings in likely unintended ways, causing undefined behavior (unexpected return values), segfaults or other memory related crashes. Fixes: - Socket descriptor (`sd`) is now initialized with -1 - `in` and `out` buffers are initialized with `nullptr` - Defensive deallocation and cleanup of buffers: * Check buffers before freeing * Zero memory before freeing * Set pointers to `nullptr` after freeing Signed-off-by: Kevin Vu te Laar <vu.te@rwth-aachen.de>
1 parent 59df604 commit 7f12fb0

2 files changed

Lines changed: 121 additions & 194 deletions

File tree

‎lib/nodes/socket.cpp‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ int villas::node::socket_init(NodeCompat *n) {
6666
auto *s = n->getData<struct Socket>();
6767

6868
s->formatter = nullptr;
69+
s->sd = -1;
70+
s->in.buf = nullptr;
71+
s->out.buf = nullptr;
6972

7073
return 0;
7174
}
@@ -76,6 +79,17 @@ int villas::node::socket_destroy(NodeCompat *n) {
7679
if (s->formatter)
7780
delete s->formatter;
7881

82+
if (s->in.buf != nullptr) {
83+
memset(s->in.buf, 0, s->in.buflen);
84+
delete[] s->in.buf;
85+
s->in.buf = nullptr;
86+
}
87+
if (s->out.buf != nullptr) {
88+
memset(s->out.buf, 0, s->out.buflen);
89+
delete[] s->out.buf;
90+
s->out.buf = nullptr;
91+
}
92+
7993
return 0;
8094
}
8195

@@ -354,8 +368,16 @@ int villas::node::socket_stop(NodeCompat *n) {
354368
return ret;
355369
}
356370

357-
delete[] s->in.buf;
358-
delete[] s->out.buf;
371+
if (s->in.buf != nullptr) {
372+
memset(s->in.buf, 0, s->in.buflen);
373+
delete[] s->in.buf;
374+
s->in.buf = nullptr;
375+
}
376+
if (s->out.buf != nullptr) {
377+
memset(s->out.buf, 0, s->out.buflen);
378+
delete[] s->out.buf;
379+
s->out.buf = nullptr;
380+
}
359381

360382
return 0;
361383
}

‎tests/unit/python_binding.py‎

Lines changed: 97 additions & 192 deletions
Original file line numberDiff line numberDiff line change
@@ -20,162 +20,147 @@ def setUp(self):
2020
except Exception as e:
2121
self.fail(f"new_node err: {e}")
2222

23-
def test_activity_changes(self):
23+
def tearDown(self):
24+
try:
25+
vn.node_stop(self.test_node)
26+
vn.node_destroy(self.test_node)
27+
except Exception as e:
28+
self.fail(f"node cleanup error: {e}")
29+
30+
def test_start(self):
2431
try:
25-
vn.node_check(self.test_node)
26-
vn.node_prepare(self.test_node)
27-
# starting twice
2832
self.assertEqual(0, vn.node_start(self.test_node))
33+
# socket node will cause a RuntimeError
34+
# the behavior is not consistent for each node
2935
# with self.assertRaises((AssertionError, RuntimeError)):
3036
# vn.node_start(self.test_node)
37+
except Exception as e:
38+
self.fail(f"err: {e}")
39+
40+
def test_new(self):
41+
try:
42+
node_uuid = str(uuid.uuid4())
43+
node_config = json.dumps(test_node_config, indent=2)
44+
node = vn.node_new(node_uuid, node_config)
45+
self.assertIsNotNone(node)
46+
except Exception as e:
47+
self.fail(f"err: {e}")
48+
49+
def test_check(self):
50+
try:
51+
vn.node_check(self.test_node)
52+
except Exception as e:
53+
self.fail(f"err: {e}")
3154

32-
# check if the node is running
55+
def test_prepare(self):
56+
try:
57+
vn.node_prepare(self.test_node)
58+
except Exception as e:
59+
self.fail(f"err: {e}")
60+
61+
def test_is_enabled(self):
62+
try:
3363
self.assertTrue(vn.node_is_enabled(self.test_node))
64+
except Exception as e:
65+
self.fail(f"err: {e}")
3466

35-
# pausing twice
36-
self.assertEqual(0, vn.node_pause(self.test_node))
67+
def test_pause(self):
68+
try:
69+
self.assertEqual(-1, vn.node_pause(self.test_node))
3770
self.assertEqual(-1, vn.node_pause(self.test_node))
71+
except Exception as e:
72+
self.fail(f"err: {e}")
3873

39-
# resuming
74+
def test_resume(self):
75+
try:
4076
self.assertEqual(0, vn.node_resume(self.test_node))
77+
except Exception as e:
78+
self.fail(f"err: {e}")
4179

42-
# stopping twice
80+
def test_stop(self):
81+
try:
4382
self.assertEqual(0, vn.node_stop(self.test_node))
44-
# self.assertEqual(-1, vn.node_stop(self.test_node))
45-
46-
# # restarting
47-
# vn.node_restart(self.test_node)
48-
49-
# check if everything still works after restarting
50-
# vn.node_pause(self.test_node)
51-
# vn.node_resume(self.test_node)
52-
# vn.node_stop(self.test_node)
83+
self.assertEqual(0, vn.node_stop(self.test_node))
84+
vn.node_restart(self.test_node)
85+
except Exception as e:
86+
self.fail(f"err: {e}")
5387

54-
# terminating the node
55-
vn.node_destroy(self.test_node)
88+
def test_restart(self):
89+
try:
90+
self.assertEqual(0, vn.node_restart(self.test_node))
91+
self.assertEqual(0, vn.node_restart(self.test_node))
5692
except Exception as e:
57-
self.fail(f" err: {e}")
93+
self.fail(f"err: {e}")
5894

59-
def test_details(self):
95+
def test_node_name(self):
6096
try:
6197
# remove color codes before checking for equality
6298
self.assertEqual(
6399
"test_node(socket)",
64100
re.sub(r"\x1b\[[0-9;]*m", "", vn.node_name(self.test_node)),
65101
)
102+
except Exception as e:
103+
self.fail(f"err: {e}")
66104

67-
# node_name_short is bugged
68-
# self.assertEqual('', vn.node_name_short(self.test_node))
105+
def test_node_name_short(self):
106+
try:
107+
print()
108+
print(f"node name short: {vn.node_name_short(self.test_node)}")
109+
print()
110+
self.assertEqual("test_node", vn.node_name_short(self.test_node))
111+
except Exception as e:
112+
self.fail(f"err: {e}")
113+
114+
def test_node_name_full(self):
115+
try:
69116
self.assertEqual(
70-
vn.node_name(self.test_node)
117+
"test_node(socket)"
71118
+ ": uuid="
72119
+ self.node_uuid
73120
+ ", #in.signals=1/1, #in.hooks=0, #out.hooks=0, in.vectorize=1, out.vectorize=1, out.netem=no, layer=udp, in.address=0.0.0.0:12000, out.address=127.0.0.1:12001",
74-
vn.node_name_full(self.test_node),
121+
re.sub(r"\x1b\[[0-9;]*m", "", vn.node_name_full(self.test_node)),
75122
)
123+
except Exception as e:
124+
self.fail(f"err: {e}")
76125

126+
def test_details(self):
127+
try:
77128
self.assertEqual(
78129
"layer=udp, in.address=0.0.0.0:12000, out.address=127.0.0.1:12001",
79130
vn.node_details(self.test_node),
80131
)
132+
except Exception as e:
133+
self.fail(f"err: {e}")
81134

135+
def test_input_signals_max_cnt(self):
136+
try:
82137
self.assertEqual(1, vn.node_input_signals_max_cnt(self.test_node))
83-
self.assertEqual(0, vn.node_output_signals_max_cnt(self.test_node))
84138
except Exception as e:
85-
self.fail(f" err: {e}")
139+
self.fail(f"err: {e}")
86140

87-
# Test whether or not the json object is created by the wrapper and the <json> module
88-
def test_json_obj(self):
141+
def test_node_output_signals_max_cnt(self):
89142
try:
90-
node_config = vn.node_to_json(self.test_node)
91-
node_config_str = json.dumps(node_config)
92-
node_config_parsed = json.loads(node_config_str)
93-
94-
self.assertEqual(node_config, node_config_parsed)
143+
self.assertEqual(0, vn.node_output_signals_max_cnt(self.test_node))
95144
except Exception as e:
96-
self.fail(f" err: {e}")
145+
self.fail(f"err: {e}")
97146

98-
# Test whether or not a node can be recreated with the string from node_to_json_str
99-
# node_to_json_str has a wrong config format, thus the name config string will create, as of now,
100-
# a node without a name
101-
# uuid can not match
102-
def test_config_from_string(self):
147+
def test_node_is_valid_name(self):
103148
try:
104-
config_str = vn.node_to_json_str(self.test_node)
105-
config_obj = json.loads(config_str)
106-
107-
config_copy_str = json.dumps(config_obj, indent=2)
108-
109-
test_node = vn.node_new("", config_copy_str)
110-
111-
self.assertEqual(
112-
re.sub(
113-
r"^[^:]+: uuid=[0-9a-fA-F-]+, ", "", vn.node_name_full(test_node)
114-
),
115-
re.sub(
116-
r"^[^:]+: uuid=[0-9a-fA-F-]+, ",
117-
"",
118-
vn.node_name_full(self.test_node),
119-
),
120-
)
149+
self.assertFalse(vn.node_is_valid_name(""))
150+
self.assertFalse(vn.node_is_valid_name("###"))
151+
self.assertFalse(vn.node_is_valid_name("v@l:d T3xt w;th invalid symb#ls"))
152+
self.assertFalse(vn.node_is_valid_name("33_characters_long_string_invalid"))
153+
self.assertTrue(vn.node_is_valid_name("32_characters_long_strings_valid"))
154+
self.assertTrue(vn.node_is_valid_name("valid_name"))
121155
except Exception as e:
122-
self.fail(f" err: {e}")
156+
self.fail(f"err: {e}")
123157

124-
def test_rw_socket(self):
158+
def test_reverse(self):
125159
try:
126-
data_str = json.dumps(send_recv_test, indent=2)
127-
data = json.loads(data_str)
128-
129-
test_nodes = {}
130-
for name, content in data.items():
131-
obj = {name: content}
132-
config = json.dumps(obj, indent=2)
133-
id = str(uuid.uuid4())
134-
135-
test_nodes[name] = vn.node_new(id, config)
136-
137-
for node in test_nodes.values():
138-
if vn.node_check(node):
139-
raise RuntimeError("Failed to verify node configuration")
140-
if vn.node_prepare(node):
141-
raise RuntimeError(
142-
f"Failed to verify {vn.node_name(node)} node configuration"
143-
)
144-
vn.node_start(node)
145-
146-
# Arrays to store samples
147-
send_smpls = vn.smps_array(1)
148-
intmdt_smpls = vn.smps_array(100)
149-
recv_smpls = vn.smps_array(100)
150-
151-
for i in range(100):
152-
send_smpls[0] = vn.sample_alloc(2)
153-
intmdt_smpls[i] = vn.sample_alloc(2)
154-
recv_smpls[i] = vn.sample_alloc(2)
155-
156-
# Generate signals and send over send_socket
157-
self.assertEqual(
158-
vn.node_read(test_nodes["signal_generator"], send_smpls, 1), 1
159-
)
160-
self.assertEqual(
161-
vn.node_write(test_nodes["send_socket"], send_smpls, 1), 1
162-
)
163-
164-
# read received signals and send them to recv_socket
165-
self.assertEqual(
166-
vn.node_read(test_nodes["intmdt_socket"], intmdt_smpls, 100), 100
167-
)
168-
self.assertEqual(
169-
vn.node_write(test_nodes["intmdt_socket"], intmdt_smpls, 100), 100
170-
)
171-
172-
# confirm rev_socket signals
173-
self.assertEqual(
174-
vn.node_read(test_nodes["recv_socket"], recv_smpls, 100), 100
175-
)
176-
160+
self.assertEqual(0, vn.node_reverse(self.test_node))
161+
self.assertEqual(0, vn.node_reverse(self.test_node))
177162
except Exception as e:
178-
self.fail(f" err: {e}")
163+
self.fail(f"err: {e}")
179164

180165

181166
test_node_config = {
@@ -191,85 +176,5 @@ def test_rw_socket(self):
191176
}
192177
}
193178

194-
send_recv_test = {
195-
"send_socket": {
196-
"type": "socket",
197-
"format": "protobuf",
198-
"layer": "udp",
199-
"in": {
200-
"address": "127.0.0.1:65532",
201-
"signals": [
202-
{"name": "voltage", "type": "float", "unit": "V"},
203-
{"name": "current", "type": "float", "unit": "A"},
204-
],
205-
},
206-
"out": {
207-
"address": "127.0.0.1:65533",
208-
"netem": {"enabled": False},
209-
"multicast": {"enabled": False},
210-
},
211-
},
212-
"intmdt_socket": {
213-
"type": "socket",
214-
"format": "protobuf",
215-
"layer": "udp",
216-
"in": {
217-
"address": "127.0.0.1:65533",
218-
"signals": [
219-
{"name": "voltage", "type": "float", "unit": "V"},
220-
{"name": "current", "type": "float", "unit": "A"},
221-
],
222-
},
223-
"out": {
224-
"address": "127.0.0.1:65534",
225-
"netem": {"enabled": False},
226-
"multicast": {"enabled": False},
227-
},
228-
},
229-
"recv_socket": {
230-
"type": "socket",
231-
"format": "protobuf",
232-
"layer": "udp",
233-
"in": {
234-
"address": "127.0.0.1:65534",
235-
"signals": [
236-
{"name": "voltage", "type": "float", "unit": "V"},
237-
{"name": "current", "type": "float", "unit": "A"},
238-
],
239-
},
240-
"out": {
241-
"address": "127.0.0.1:65535",
242-
"netem": {"enabled": False},
243-
"multicast": {"enabled": False},
244-
},
245-
},
246-
"signal_generator": {
247-
"type": "signal.v2",
248-
"limit": 100,
249-
"rate": 10,
250-
"in": {
251-
"signals": [
252-
{
253-
"amplitude": 2,
254-
"name": "voltage",
255-
"phase": 90,
256-
"signal": "sine",
257-
"type": "float",
258-
"unit": "V",
259-
},
260-
{
261-
"amplitude": 1,
262-
"name": "current",
263-
"phase": 0,
264-
"signal": "sine",
265-
"type": "float",
266-
"unit": "A",
267-
},
268-
],
269-
"hooks": [{"type": "print", "format": "villas.human"}],
270-
},
271-
},
272-
}
273-
274179
if __name__ == "__main__":
275180
unittest.main()

0 commit comments

Comments
 (0)