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

[pulsar-next] Pulsar sometimes crashes on reload #1215

Open
5 tasks done
Purple-Fox-Coder opened this issue Feb 11, 2025 · 3 comments
Open
5 tasks done

[pulsar-next] Pulsar sometimes crashes on reload #1215

Purple-Fox-Coder opened this issue Feb 11, 2025 · 3 comments
Labels
bug Something isn't working pulsar-next Related to the version of Pulsar that runs on the latest Electron

Comments

@Purple-Fox-Coder
Copy link

Thanks in advance for your bug report!

  • Have you reproduced issue in safe mode?
  • Have you used the debugging guide to try to resolve the issue?
  • Have you checked our FAQs to make sure your question isn't answered there?
  • Have you checked to make sure your issue does not already exist?
  • Have you checked you are on the latest release of Pulsar?

What happened?

Image

Pulsar version

1.124.2025020900-next

Which OS does this happen on?

🪟 Windows

OS details

Windows 11

Which CPU architecture are you running this on?

x86_64/AMD64

What steps are needed to reproduce this?

  1. Launch pulsar-next
  2. press CTRL+SHIFT+F5 (reload keybind)

Additional Information:

No response

@Purple-Fox-Coder Purple-Fox-Coder added the bug Something isn't working label Feb 11, 2025
@savetheclocktower
Copy link
Contributor

OK, I dipped my toes into this one today by updating my dev environment on Windows and running yarn install and yarn build.

I'm able to reproduce the crash on the second window reload (typically), so the next step is finding the crash dump data for analysis. This article looks great, except that it says my crash dumps will be in a location where they are not.

When I get more time to work on this, I'll hopefully get as far as figuring out where they're being stored. (If they're being stored.) So far I can ascertain it's not in %LOCALAPPDATA%\CrashDumps and seems not to be anywhere below $ATOM_HOME.

@savetheclocktower
Copy link
Contributor

savetheclocktower commented Feb 14, 2025

Amazingly, I think this already got fixed because of work I did ~6 months ago.

To figure out what was crashing on Windows, I dropped in the Sentry integration I'd used successfully on macOS, then reloaded until I got a crash; I think it even happened on the first try. I was able to ascertain that that crash was happening somewhere inside nsfw, the module that we use for most of our file-watching.

My understanding is that, when a window “reloads” in Electron, the teardown of one page overlaps with the setup of the next. Each page load is a separate context in the same process, much like worker threads that use the same library. (This is exactly why modules need to be context-aware to be used in an Electron renderer process — native code will be loaded once but will be called from multiple contexts and cannot share state.)

But context-awareness wasn't the problem here. Imagine that each page is the equivalent of a Node worker thread. In this case, what tripped up nsfw was the fact that Electron apparently terminates the worker (rather than signaling it should exit and waiting for it to do so on its own time). If, somehow, we try to start a new file-watcher while the environment is terminating, that causes an exception in nsfw; the fact that it's terminating means that no cleanup code has been run, so the finalizers that would ordinarily allow nsfw to detect that it's shutting down are bypassed. Hence nsfw itself has no ability to prevent this exception.

Luckily, N-API has envisioned this situation. I contributed this fix to nsfw 5 months ago — it sets a build flag so that N-API knows to swallow exceptions in this situation instead of allowing them to crash the process.

Why wasn't Pulsar already using this fix? Because it's also a bug that Pulsar tries to create a new file watcher in the midst of a terminating environment, and I thought I'd been able to prevent the crash by being more thoughtful about file-watcher cleanup — hence I didn't have an immediate need to update to the fixed nsfw; I probably thought I'd be fine to wait until the next official release. But it's been 5 months without a new release, so it's time to install nsfw directly from GitHub.

After installing the latest nsfw from master, I was able to do a window reload five times in a row without any crashes, so I'm claiming victory. It's not great to find out that something triggered this crash — presumably by starting a new listener in a terminating environment, that thing I thought I had fixed ages ago — but I can track that down later. (This fix would always have been needed eventually, since we expose a file-watcher API via the package API; I can control what Pulsar itself does, but not what community packages do.)

@Purple-Fox-Coder, another rolling release binary should be available soon; let me know if it solves the issue for you. If so, I can close this ticket.

@savetheclocktower
Copy link
Contributor

(The fix is in cf06e8e.)

@savetheclocktower savetheclocktower added the pulsar-next Related to the version of Pulsar that runs on the latest Electron label Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pulsar-next Related to the version of Pulsar that runs on the latest Electron
Projects
None yet
Development

No branches or pull requests

2 participants