Download raw body.
tog: lock mutex on script error + blame view deallocation tweak
Below are two diffs. Both of which apply on top of the pending blame
regress diff, but they _might_ apply without it.
First is short and simple: while working on something else, I noticed
that we fail to relock the mutex if we return from view_loop() early
due to a tog_read_script_key() error.
diff /home/mark/src/got
commit - 6419890e5180822b712d47c23cc4d30e63966085
path + /home/mark/src/got
blob - 232ab4c8aa553e2c4cf0a880fe27470a40b63e0f
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -1707,8 +1707,10 @@ view_input(struct tog_view **new, int *done, struct to
if (using_mock_io) {
err = tog_read_script_key(tog_io.f, &ch, done);
- if (err)
+ if (err) {
+ errcode = pthread_mutex_lock(&tog_mutex);
return err;
+ }
} else if (view->count && --view->count) {
cbreak();
nodelay(view->window, TRUE);
Next, while more closely studying the recent view leak fix in 2ca2f982e,
I realised we need to treat the blame view case a little differently.
Specifically, we only want to free the tog_view structure if
open_blame_view() fails--not on any error case in cmd_blame() because if
we make it to a view_loop() call in cmd_blame(), view gets freed at the
end of view_loop() irrespective of error. And apparently simply setting
view to NULL inside view_close() wasn't enough for the (view != NULL)
guard. So I've moved view_open() to immediately before the
open_blame_view() call, and moved the view->close/view_close() calls to
immediately after it so they are only called when open_blame_view()
returns an error.
The above is not a problem in the other views, because we are calling
view_close/view->close from the open_*_view() routines--not cmd_*();
that is, view_loop() is never entered in those cases.
I've attached a diff(1) patch of this because 'got diff' doesn't show
this change as cleanly.
--- tog.c Sat Apr 15 22:29:33 2023
+++ tog/tog.c Sat Apr 15 22:25:51 2023
@@ -1707,8 +1707,10 @@ view_input(struct tog_view **new, int *done, struct to
if (using_mock_io) {
err = tog_read_script_key(tog_io.f, &ch, done);
- if (err)
+ if (err) {
+ errcode = pthread_mutex_lock(&tog_mutex);
return err;
+ }
} else if (view->count && --view->count) {
cbreak();
nodelay(view->window, TRUE);
@@ -6997,12 +6999,6 @@ cmd_blame(int argc, char *argv[])
if (error != NULL)
goto done;
- view = view_open(0, 0, 0, 0, TOG_VIEW_BLAME);
- if (view == NULL) {
- error = got_error_from_errno("view_open");
- goto done;
- }
-
error = got_object_open_as_commit(&commit, repo, commit_id);
if (error)
goto done;
@@ -7012,10 +7008,19 @@ cmd_blame(int argc, char *argv[])
if (error)
goto done;
+ view = view_open(0, 0, 0, 0, TOG_VIEW_BLAME);
+ if (view == NULL) {
+ error = got_error_from_errno("view_open");
+ goto done;
+ }
error = open_blame_view(view, link_target ? link_target : in_repo_path,
commit_id, repo);
- if (error)
+ if (error != NULL) {
+ if (view->close == NULL)
+ close_blame_view(view);
+ view_close(view);
goto done;
+ }
if (worktree) {
/* Release work tree lock. */
got_worktree_close(worktree);
@@ -7028,11 +7033,6 @@ done:
free(link_target);
free(cwd);
free(commit_id);
- if (error != NULL && view != NULL) {
- if (view->close == NULL)
- close_blame_view(view);
- view_close(view);
- }
if (commit)
got_object_commit_close(commit);
if (worktree)
--
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
tog: lock mutex on script error + blame view deallocation tweak