Skip to content

buffer: Remove buffer invalidation in CloseOpenBuffers()#4133

Open
JoeKar wants to merge 1 commit into
micro-editor:masterfrom
JoeKar:fix/4131
Open

buffer: Remove buffer invalidation in CloseOpenBuffers()#4133
JoeKar wants to merge 1 commit into
micro-editor:masterfrom
JoeKar:fix/4131

Conversation

@JoeKar

@JoeKar JoeKar commented Jun 29, 2026

Copy link
Copy Markdown
Member

The invalidation was introduced with #2898 to workaround the same root cause:
OpenBuffers is modified in Close() while the iteration is ongoing. This causes
not closed buffers.
We solve this situation by using a local copy of OpenBuffers to do the necessary
iteration. The modification is still done by Close().
Additional functions don't have to check for nil'ed buffers.

Fixes #4131

@JoeKar JoeKar changed the title buffer: Ignore already invalidated buffers in Shared() buffer: Remove buffer invalidation in CloseOpenBuffers() Jul 7, 2026
The invalidation was introduced with micro-editor#2898 to workaround the same root cause:
OpenBuffers is modified in Close() while the iteration is ongoing. This causes
not closed buffers.
We solve this situation by using a local copy of OpenBuffers to do the necessary
iteration. The modification is still done by Close().
Additional functions don't have to check for nil'ed buffers.
Comment thread internal/buffer/buffer.go
@@ -502,11 +502,14 @@ func NewBuffer(r io.Reader, size int64, path string, btype BufType, cmd Command)

@dmaluka dmaluka Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The commit message, again, sounds confusing:

to workaround the same root cause

The root cause of what? The same as what?

The modification is still done by Close().

"still"? Before this commit it wasn't done by Close(), was it?

Why not clearly say that this fixes #4131, and in order to avoid reintroducing the issue fixed by #2898, uses a local copy?

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.

QuitAll crashes micro

2 participants