Relax commit methods to &self in database

[helloskyline-a11y]

Closes #182. I am using my main account to make these commits.

"mostly due to the requirements of the bindings if I remember correctly"

You were right. By relaxing &mut self to &self on the Rust side, JS bindings didn't fail, but it ultimately triggered a SIGSEGV at the very end of the Jest run. I alos fixed a missing await on db.addHistoricEvents in the nul-bytes test which was causing, I believe, a race condition.

I ran all tests, and they all passed, with no more SIGSEGV. clippy fails but it is already patched on #189 (d397899).

[helloskyline-a11y]

I also noticed reload can switch to &self too, but I don't think it is a good idea to do so.

[helloskyline-a11y]

After careful re-reading, I extracted a good quote but with the wrong context 🤣
Still, it is very pertinent but kinda funny.

Edit: I also want to open another PR to avoid a JS test to update data directory. This would make tests idempotent. I had to rollback this directory during my tests.

[poljar]

CI please start?

[poljar]

You were right. By relaxing &mut self to &self on the Rust side, JS bindings didn't fail, but it ultimately triggered a SIGSEGV at the very end of the Jest run.

What fixed the SIGSEGV?

[helloskyline-a11y]

You were right. By relaxing &mut self to &self on the Rust side, JS bindings didn't fail, but it ultimately triggered a SIGSEGV at the very end of the Jest run.

What fixed the SIGSEGV?

Sorry for the delay, I was not at home.
SIGSEGV was caused by a missing await on db.addHistoricEvents(events) call in the nul-bytes test (L366). Because the Promise returned by addHistoricEvents was not awaited, Jest completed the test execution and began tearing down the environment.

I believe it was a missing from the early beginning since you are using addHistoricEvents with await on "allow messages from the backlog" test but not in the nul-bytes test. While you were using addHistoricEventsSync in some others.

[helloskyline-a11y]

Old message

When I take a closer look, I am surprised that SIGSEGV disappeared. But it weirdly does.

Edit: I understand why and made a patch. I'm gonna eat and push everything with an explanation.

SIGSEGV was caused by a race condition. It became possible after relaxing API from &mut self to &self (my bad). I changed binding from borrow_mut() to borrow() (but it'd have been the same); this allowed JS to call reload() while commit_no_wait() was still running its task on a background thread. I belive because tantivy relies on mmap files, modifying the index on the background thread while the main thread attempted to reload the files caused the SIGSEGV.

Actually, adding the missing await on the JS side resolved the crash because it forced the JS execution to pause until the background thread finished its task.

The solution I propose is just to integrate the reload into mpsc. That way, even if we make ten million commits and then run reload, there won't be any race conditions, because reload will be executed after all the commits. I'd like to hear your thoughts, though. I've checked several times to see if this update fixes the bug, and it appears that it does (even withtout the await). However, since reload should only be used in tests, I think await is still sufficient. But both are really small changes, even if first proposal (with mpsc) looks pretty elegant.

[helloskyline-a11y]

Concerning CI, I think it's because I try to merge on main while ci.yml#L8 only run on branches: [master]. zizmor.yml looks more permissive. Maybe it's the cause.

Edit: my PGP key expired... "git error: gpg failed to sign the data", byecame back to life 🤘