jarun / nnn

n³ The unorthodox terminal file manager
BSD 2-Clause "Simplified" License
18.51k stars 743 forks source link

malloc: error pointer being freed was not allocated #1877

Closed XhstormR closed 2 months ago

XhstormR commented 2 months ago

Environment details (Put x in the checkbox along with the information)

Exact steps to reproduce the issue

nnn 4.9 installed from brew, after pressing n to enter nnn, directly enter ? for help, then enter q to exit help, and then press q again to exit nnn, and then an exception will occur.

Exception log:

        Alt ;  Select plugin           =  Launch app
         ! ^]  Shell                   ]  Cmd prompt
nnn(53031,0x7ff84ee82100) malloc: *** error for object 0x2020202020202020: pointer being freed was not allocated
nnn(53031,0x7ff84ee82100) malloc: *** set a breakpoint in malloc_error_break to debug
fish: Job 1, 'command nnn $argv' terminated by signal SIGABRT (Abort)
⏎
image
jarun commented 2 months ago

I am unable to reproduce on Ubuntu. Can you please debug further to figure out which pointer is being freed without allocation?

jarun commented 2 months ago

This is coming from commit 3665541dac1479a83edae016aa1053ac7be162ba and the next one.

@N-R-K can you please have a look?

jarun commented 2 months ago

Can we write directly to fd instead of having the temporary buffer?

jarun commented 2 months ago

Please confirm that the fix works.

N-R-K commented 2 months ago

This looks like a duplicate of https://github.com/jarun/nnn/issues/1768

It should be fixed in master. @XhstormR Can you try master branch and confirm?

This is coming from commit 3665541 and the next one.

@jarun I'm pretty sure that that is the commit that fixed the issue. https://github.com/jarun/nnn/commit/be6988d1c849895e850a67f0f7f5ad176415c513 should not be necessary.

N-R-K commented 2 months ago

Can we write directly to fd instead of having the temporary buffer?

Making a syscall for each and every byte seems unnecessary waste of resources to me.

IMO https://github.com/jarun/nnn/commit/be6988d1c849895e850a67f0f7f5ad176415c513 should be reverted since the bug was already fixed.

OP was using v4.9 instead of master branch. Maybe we can cut a new release soon?

jarun commented 2 months ago

I reproduced this one master (using fastmac) before attempting the fix.

As for a syscall overhead, I will add a new version to use dprintf.

N-R-K commented 2 months ago

I reproduced this one master (using fastmac) before attempting the fix.

Interesting. I don't know how that's possible. The help string is 1832 bytes and the buffer is 2048 bytes. It has more than enough space.

As for a syscall overhead, I will add a new version to use dprintf.

dprintf also makes 1 write syscall per call since dprintf uses file-descriptor directly without any buffering.

You'd need to use a FILE * with fopen + fwrite etc to avoid many syscalls. Do you want to make that change yourself or should I open a PR later today?

jarun commented 2 months ago

Please raise the PR.