Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid a deadlock updating the network D-Bus tree #966

Merged
merged 2 commits into from
Dec 29, 2023

Conversation

imobachgs
Copy link
Contributor

@imobachgs imobachgs commented Dec 29, 2023

Problem

If the network service receives an Apply at the same time as any other request, it may get blocked.

Solution

Updating the tree must be done in a separate task. Otherwise, it could fight with an incoming request for the ObjectServer mutex, blocking the actions dispatching.

Using tokio-console

I decided to use tokio-console to make it easier to find the problem. This tool collects and displays information about the asynchronous tasks in your program. In the screenshot below you can see that the D-Bus Set dispatchers are blocked.

Captura desde 2023-12-29 10-52-45

Once you find the problem, it is kind of "obvious", but it may take some time until you figure out what is happening.

To use tokio-console you need to add some instrumentation to your program and, at this point, it implies adding some unstable Tokio APIs. So if you want to give it a try, you can try the code in the tokio-console branch.

Updating the tree must be done in a separate task. Otherwise, it could
fight with an incoming request for the ObjectServer mutex, blocking the
actions dispatching.
@imobachgs imobachgs force-pushed the avoid-dbus-tree-deadlock branch from 4fc4d64 to 1c9ec66 Compare December 29, 2023 10:15
@coveralls
Copy link

coveralls commented Dec 29, 2023

Coverage Status

coverage: 74.997% (-0.01%) from 75.009%
when pulling c40e251 on avoid-dbus-tree-deadlock
into e4ae7ed on master.

Copy link
Contributor

@teclator teclator left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@teclator teclator left a comment

Choose a reason for hiding this comment

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

Even better now ;)

@imobachgs imobachgs merged commit d9b0131 into master Dec 29, 2023
2 checks passed
@imobachgs imobachgs deleted the avoid-dbus-tree-deadlock branch December 29, 2023 13:03
@imobachgs imobachgs mentioned this pull request Feb 12, 2024
@imobachgs imobachgs mentioned this pull request May 17, 2024
imobachgs added a commit that referenced this pull request May 17, 2024
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
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