diff options
author | Jaakko Keränen <jaakko.keranen@iki.fi> | 2020-11-24 22:11:11 +0200 |
---|---|---|
committer | Jaakko Keränen <jaakko.keranen@iki.fi> | 2020-11-24 22:11:11 +0200 |
commit | 5268fb9f7e1bca4b0fc496ff115c253aea724e49 (patch) | |
tree | 8023f45f3538f7966c2f75674a4b14fb4778e79c /src/ui/documentwidget.c | |
parent | 9421f64ee86e6aec26e05a45bcfc65edd895a8a8 (diff) |
Fixed threading issues and data races
The most serious problem was that GmRequest's response body was being accessed while the TlsRequest thread was modifying it.
Now the response must always be locked before accessing elsewhere.
There were also inefficient data updates in the media players.
Diffstat (limited to 'src/ui/documentwidget.c')
-rw-r--r-- | src/ui/documentwidget.c | 51 |
1 files changed, 29 insertions, 22 deletions
diff --git a/src/ui/documentwidget.c b/src/ui/documentwidget.c index d7d15a67..a843e840 100644 --- a/src/ui/documentwidget.c +++ b/src/ui/documentwidget.c | |||
@@ -766,7 +766,7 @@ static void showErrorPage_DocumentWidget_(iDocumentWidget *d, enum iGmStatusCode | |||
766 | 766 | ||
767 | static void updateFetchProgress_DocumentWidget_(iDocumentWidget *d) { | 767 | static void updateFetchProgress_DocumentWidget_(iDocumentWidget *d) { |
768 | iLabelWidget *prog = findWidget_App("document.progress"); | 768 | iLabelWidget *prog = findWidget_App("document.progress"); |
769 | const size_t dlSize = d->request ? size_Block(body_GmRequest(d->request)) : 0; | 769 | const size_t dlSize = d->request ? bodySize_GmRequest(d->request) : 0; |
770 | setFlags_Widget(as_Widget(prog), hidden_WidgetFlag, dlSize < 250000); | 770 | setFlags_Widget(as_Widget(prog), hidden_WidgetFlag, dlSize < 250000); |
771 | if (isVisible_Widget(prog)) { | 771 | if (isVisible_Widget(prog)) { |
772 | updateText_LabelWidget(prog, | 772 | updateText_LabelWidget(prog, |
@@ -1032,9 +1032,10 @@ static void checkResponse_DocumentWidget_(iDocumentWidget *d) { | |||
1032 | if (statusCode == none_GmStatusCode) { | 1032 | if (statusCode == none_GmStatusCode) { |
1033 | return; | 1033 | return; |
1034 | } | 1034 | } |
1035 | iGmResponse *resp = lockResponse_GmRequest(d->request); | ||
1035 | if (d->state == fetching_RequestState) { | 1036 | if (d->state == fetching_RequestState) { |
1036 | d->state = receivedPartialResponse_RequestState; | 1037 | d->state = receivedPartialResponse_RequestState; |
1037 | updateTrust_DocumentWidget_(d, response_GmRequest(d->request)); | 1038 | updateTrust_DocumentWidget_(d, resp); |
1038 | init_Anim(&d->sideOpacity, 0); | 1039 | init_Anim(&d->sideOpacity, 0); |
1039 | switch (category_GmStatusCode(statusCode)) { | 1040 | switch (category_GmStatusCode(statusCode)) { |
1040 | case categoryInput_GmStatusCode: { | 1041 | case categoryInput_GmStatusCode: { |
@@ -1045,9 +1046,9 @@ static void checkResponse_DocumentWidget_(iDocumentWidget *d) { | |||
1045 | as_Widget(d), | 1046 | as_Widget(d), |
1046 | NULL, | 1047 | NULL, |
1047 | format_CStr(uiHeading_ColorEscape "%s", cstr_Rangecc(parts.host)), | 1048 | format_CStr(uiHeading_ColorEscape "%s", cstr_Rangecc(parts.host)), |
1048 | isEmpty_String(meta_GmRequest(d->request)) | 1049 | isEmpty_String(&resp->meta) |
1049 | ? format_CStr("Please enter input for %s:", cstr_Rangecc(parts.path)) | 1050 | ? format_CStr("Please enter input for %s:", cstr_Rangecc(parts.path)) |
1050 | : cstr_String(meta_GmRequest(d->request)), | 1051 | : cstr_String(&resp->meta), |
1051 | uiTextCaution_ColorEscape "Send \u21d2", | 1052 | uiTextCaution_ColorEscape "Send \u21d2", |
1052 | "document.input.submit"); | 1053 | "document.input.submit"); |
1053 | setSensitive_InputWidget(findChild_Widget(dlg, "input"), | 1054 | setSensitive_InputWidget(findChild_Widget(dlg, "input"), |
@@ -1057,15 +1058,15 @@ static void checkResponse_DocumentWidget_(iDocumentWidget *d) { | |||
1057 | case categorySuccess_GmStatusCode: | 1058 | case categorySuccess_GmStatusCode: |
1058 | init_Anim(&d->scrollY, 0); | 1059 | init_Anim(&d->scrollY, 0); |
1059 | reset_GmDocument(d->doc); /* new content incoming */ | 1060 | reset_GmDocument(d->doc); /* new content incoming */ |
1060 | updateDocument_DocumentWidget_(d, response_GmRequest(d->request), iTrue); | 1061 | updateDocument_DocumentWidget_(d, resp, iTrue); |
1061 | break; | 1062 | break; |
1062 | case categoryRedirect_GmStatusCode: | 1063 | case categoryRedirect_GmStatusCode: |
1063 | if (isEmpty_String(meta_GmRequest(d->request))) { | 1064 | if (isEmpty_String(&resp->meta)) { |
1064 | showErrorPage_DocumentWidget_(d, invalidRedirect_GmStatusCode, NULL); | 1065 | showErrorPage_DocumentWidget_(d, invalidRedirect_GmStatusCode, NULL); |
1065 | } | 1066 | } |
1066 | else { | 1067 | else { |
1067 | /* Only accept redirects that use gemini scheme. */ | 1068 | /* Only accept redirects that use gemini scheme. */ |
1068 | const iString *dstUrl = absoluteUrl_String(d->mod.url, meta_GmRequest(d->request)); | 1069 | const iString *dstUrl = absoluteUrl_String(d->mod.url, &resp->meta); |
1069 | if (d->redirectCount >= 5) { | 1070 | if (d->redirectCount >= 5) { |
1070 | showErrorPage_DocumentWidget_(d, tooManyRedirects_GmStatusCode, dstUrl); | 1071 | showErrorPage_DocumentWidget_(d, tooManyRedirects_GmStatusCode, dstUrl); |
1071 | } | 1072 | } |
@@ -1085,17 +1086,17 @@ static void checkResponse_DocumentWidget_(iDocumentWidget *d) { | |||
1085 | break; | 1086 | break; |
1086 | default: | 1087 | default: |
1087 | if (isDefined_GmError(statusCode)) { | 1088 | if (isDefined_GmError(statusCode)) { |
1088 | showErrorPage_DocumentWidget_(d, statusCode, meta_GmRequest(d->request)); | 1089 | showErrorPage_DocumentWidget_(d, statusCode, &resp->meta); |
1089 | } | 1090 | } |
1090 | else if (category_GmStatusCode(statusCode) == | 1091 | else if (category_GmStatusCode(statusCode) == |
1091 | categoryTemporaryFailure_GmStatusCode) { | 1092 | categoryTemporaryFailure_GmStatusCode) { |
1092 | showErrorPage_DocumentWidget_( | 1093 | showErrorPage_DocumentWidget_( |
1093 | d, temporaryFailure_GmStatusCode, meta_GmRequest(d->request)); | 1094 | d, temporaryFailure_GmStatusCode, &resp->meta); |
1094 | } | 1095 | } |
1095 | else if (category_GmStatusCode(statusCode) == | 1096 | else if (category_GmStatusCode(statusCode) == |
1096 | categoryPermanentFailure_GmStatusCode) { | 1097 | categoryPermanentFailure_GmStatusCode) { |
1097 | showErrorPage_DocumentWidget_( | 1098 | showErrorPage_DocumentWidget_( |
1098 | d, permanentFailure_GmStatusCode, meta_GmRequest(d->request)); | 1099 | d, permanentFailure_GmStatusCode, &resp->meta); |
1099 | } | 1100 | } |
1100 | break; | 1101 | break; |
1101 | } | 1102 | } |
@@ -1104,12 +1105,13 @@ static void checkResponse_DocumentWidget_(iDocumentWidget *d) { | |||
1104 | switch (category_GmStatusCode(statusCode)) { | 1105 | switch (category_GmStatusCode(statusCode)) { |
1105 | case categorySuccess_GmStatusCode: | 1106 | case categorySuccess_GmStatusCode: |
1106 | /* More content available. */ | 1107 | /* More content available. */ |
1107 | updateDocument_DocumentWidget_(d, response_GmRequest(d->request), iFalse); | 1108 | updateDocument_DocumentWidget_(d, resp, iFalse); |
1108 | break; | 1109 | break; |
1109 | default: | 1110 | default: |
1110 | break; | 1111 | break; |
1111 | } | 1112 | } |
1112 | } | 1113 | } |
1114 | unlockResponse_GmRequest(d->request); | ||
1113 | } | 1115 | } |
1114 | 1116 | ||
1115 | static const char *sourceLoc_DocumentWidget_(const iDocumentWidget *d, iInt2 pos) { | 1117 | static const char *sourceLoc_DocumentWidget_(const iDocumentWidget *d, iInt2 pos) { |
@@ -1190,18 +1192,21 @@ static iBool handleMediaCommand_DocumentWidget_(iDocumentWidget *d, const char * | |||
1190 | /* Pass new data to media players. */ | 1192 | /* Pass new data to media players. */ |
1191 | const enum iGmStatusCode code = status_GmRequest(req->req); | 1193 | const enum iGmStatusCode code = status_GmRequest(req->req); |
1192 | if (isSuccess_GmStatusCode(code)) { | 1194 | if (isSuccess_GmStatusCode(code)) { |
1193 | if (startsWith_String(meta_GmRequest(req->req), "audio/")) { | 1195 | iGmResponse *resp = lockResponse_GmRequest(req->req); |
1196 | if (startsWith_String(&resp->meta, "audio/")) { | ||
1194 | /* TODO: Use a helper? This is same as below except for the partialData flag. */ | 1197 | /* TODO: Use a helper? This is same as below except for the partialData flag. */ |
1195 | setData_Media(media_GmDocument(d->doc), | 1198 | if (setData_Media(media_GmDocument(d->doc), |
1196 | req->linkId, | 1199 | req->linkId, |
1197 | meta_GmRequest(req->req), | 1200 | &resp->meta, |
1198 | body_GmRequest(req->req), | 1201 | &resp->body, |
1199 | partialData_MediaFlag | allowHide_MediaFlag); | 1202 | partialData_MediaFlag | allowHide_MediaFlag)) { |
1200 | redoLayout_GmDocument(d->doc); | 1203 | redoLayout_GmDocument(d->doc); |
1204 | } | ||
1201 | updateVisible_DocumentWidget_(d); | 1205 | updateVisible_DocumentWidget_(d); |
1202 | invalidate_DocumentWidget_(d); | 1206 | invalidate_DocumentWidget_(d); |
1203 | refresh_Widget(as_Widget(d)); | 1207 | refresh_Widget(as_Widget(d)); |
1204 | } | 1208 | } |
1209 | unlockResponse_GmRequest(req->req); | ||
1205 | } | 1210 | } |
1206 | /* Update the link's progress. */ | 1211 | /* Update the link's progress. */ |
1207 | invalidateLink_DocumentWidget_(d, req->linkId); | 1212 | invalidateLink_DocumentWidget_(d, req->linkId); |
@@ -1482,8 +1487,9 @@ static iBool handleCommand_DocumentWidget_(iDocumentWidget *d, const char *cmd) | |||
1482 | return iTrue; | 1487 | return iTrue; |
1483 | } | 1488 | } |
1484 | else if (equalWidget_Command(cmd, w, "document.request.updated") && | 1489 | else if (equalWidget_Command(cmd, w, "document.request.updated") && |
1485 | pointerLabel_Command(cmd, "request") == d->request) { | 1490 | d->request && pointerLabel_Command(cmd, "request") == d->request) { |
1486 | set_Block(&d->sourceContent, body_GmRequest(d->request)); | 1491 | set_Block(&d->sourceContent, &lockResponse_GmRequest(d->request)->body); |
1492 | unlockResponse_GmRequest(d->request); | ||
1487 | if (document_App() == d) { | 1493 | if (document_App() == d) { |
1488 | updateFetchProgress_DocumentWidget_(d); | 1494 | updateFetchProgress_DocumentWidget_(d); |
1489 | } | 1495 | } |
@@ -1501,7 +1507,8 @@ static iBool handleCommand_DocumentWidget_(iDocumentWidget *d, const char *cmd) | |||
1501 | /* The response may be cached. */ { | 1507 | /* The response may be cached. */ { |
1502 | if (!equal_Rangecc(urlScheme_String(d->mod.url), "about") && | 1508 | if (!equal_Rangecc(urlScheme_String(d->mod.url), "about") && |
1503 | startsWithCase_String(meta_GmRequest(d->request), "text/")) { | 1509 | startsWithCase_String(meta_GmRequest(d->request), "text/")) { |
1504 | setCachedResponse_History(d->mod.history, response_GmRequest(d->request)); | 1510 | setCachedResponse_History(d->mod.history, lockResponse_GmRequest(d->request)); |
1511 | unlockResponse_GmRequest(d->request); | ||
1505 | } | 1512 | } |
1506 | } | 1513 | } |
1507 | iReleasePtr(&d->request); | 1514 | iReleasePtr(&d->request); |
@@ -2496,7 +2503,7 @@ static void drawRun_DrawContext_(void *context, const iGmRun *run) { | |||
2496 | topRight_Rect(linkRect), | 2503 | topRight_Rect(linkRect), |
2497 | tmInlineContentMetadata_ColorId, | 2504 | tmInlineContentMetadata_ColorId, |
2498 | " \u2014 Fetching\u2026 (%.1f MB)", | 2505 | " \u2014 Fetching\u2026 (%.1f MB)", |
2499 | (float) size_Block(body_GmRequest(mr->req)) / 1.0e6f); | 2506 | (float) bodySize_GmRequest(mr->req) / 1.0e6f); |
2500 | } | 2507 | } |
2501 | } | 2508 | } |
2502 | else if (isHover) { | 2509 | else if (isHover) { |