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

fix #345 #349

Closed
wants to merge 1 commit into from
Closed

fix #345 #349

wants to merge 1 commit into from

Conversation

liquidaty
Copy link
Owner

No description provided.

@liquidaty
Copy link
Owner Author

@richiejp spent a LOT of time debugging. Think I found the issue. Hope this resolves properly, but pls take a look to see:

On macos-14, upon pressing T:

  1. get_data_index_async() never fires, and there is a very brief "Unexpected error!" flash in the status bar because it tries to seek to a newline, but there is no index data so it seeks to the start of the file and does not encounter a newline

  2. This line of code only executes once, and sets need_index to false, because uibuff->write_in_progress is set to 1 via uibopts.write_after_open in transformation.c:247. Perhaps on other platforms the write_in_progress is actually not 1 due to it being a bit and uibopts.write_after_open being a char?

  • char need_index = !uibuff->index_started && !uibuff->write_in_progress;
  1. There might be a race condition from the time between need_index = uibuff->index_started and writing uibuff->index_started = 1

  2. Changing need_index to not check write_in_progress, and changing transformation.c to not explicitly call the worker. This also makes various functions in transformation.c unnecessary; they seem to be redundant with what is in index.c anyway and probably would have been best consolidated from the get-go

@liquidaty
Copy link
Owner Author

Looking more closely this definitely does not fix the issue without causing others but adding zsvsheet_run_buffer_transformation() back in causes an assertion to fail.

If it is kept, zsvsheet_run_buffer_transformation() should be consolidated with the similar code in index.c, and the mechanism for starting an index run should also be consolidated, probably triggered in read-data.c so that it does not need to come via a push_transformation call. That sound good?

@liquidaty liquidaty closed this Dec 19, 2024
@richiejp
Copy link
Contributor

get_data_index_async() never fires, and there is a very brief "Unexpected error!" flash in the status bar

Just on a partially related note, I added something which makes the status messages last longer here @liquidaty

2ed256f

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.

2 participants