BrowserProcess keeps open new window instead of restart about electron-connect HOT 34 CLOSED

Same problem. I was using a fork from Quramy's boilerplate as a starting point of my project. Electron-connect works fine there and correctly kills the process if you change a browser process coffee, the boilerplate uses ^0.1.6 as the electron-connect version. When I upgraded the electron-connect to the latest,it no longer kills the process when a change is made.

You can easily reproduce this with electron-quick-start.
Just clone the repo,npm install electron-connect and add the gulp task. It wont kill the process when a change is applied to the main.js. Here is a screenshot

I changed the browser window size and the first process is still there.

from electron-connect.

@arkenthera Are you calling electron-connect's client.create() from renderer or browser process?

I'm noticing the issue when calling it from renderer process.

from electron-connect.

Also having this issue, it started happening in the last week. Calling electron.start() from a gulpfile.

from electron-connect.

@jescalan if you are calling client.create() from renderer process, then try calling it from browser process.

The issue is - when client.create() is called from renderer process, killing the electron app (as part of the restart) terminates all the child processes, except the main process of the electron app. I believe this is happening since #35 was merged.

from electron-connect.

I'm calling it straight in a gulp task, so I don't think that's in a renderer process...

from electron-connect.

@jescalan I think you got it wrong. Typically, you call electron-connect's server start() in gulp task. But, client.create() has to be in either browser process or renderer process. And, the issue is with client.create() being called from renderer process.

from electron-connect.

Ah yeah sorry, the client.create() is being called from my main html file, which I assume is the browser process?

from electron-connect.

No. That's renderer process. Browser process is the one that has BrowserWindow() calls to create the windows (renderer processes)

from electron-connect.

Ah ok, let me try it out the other way. Thanks, and sorry for the dumb questions!

from electron-connect.

Hey, I'm not using electron-connect for a while,that said, when I posted about the issue I worked it around by modifying the source but unfortunately I can't remember what I changed back then.You can download it here.

from electron-connect.

@arkenthera, A lot has changed since then. It would be nice if you could reproduce it with the latest version and let us know your findings.

from electron-connect.

We are still experiencing this with the latest version of Electron and Electron-Connect, we would prefer not to put it inside of the browser process because this stuff is loaded conditionally and only in certain situations. Is there no way we can hook into a close action and just force close it?

from electron-connect.

Actually, we just randomly tried to debug the problem and solved this problem by just doing this in our Gulpfile which fixed the problem entirely:

electron.start(args, () => {})

It seems that adding a callback to start makes the problem go away. Buggy if you ask me but whatever, it works so we'll keep it there for the forseeable future.

from electron-connect.

@envygeeks you could conditionally load in browser process too. Not sure whats preventing you to do so. Also, are you sure the workaround is working for you? I just tried it in simple example from the repo, and it didn't work for me !!

from electron-connect.

@ssreekanth in theory we could but it would be late compared to where we load and connect right now in the main process. And yes, I am sure it closes out all other Windows because on my Linux system it would show all windows regardless of whether they have content or not, duplicate or not.

Here is how we have it setup maybe you can compare:

gulp.task("dev", ["build:dist"], (gulpCallback) => {
  events.on("build:dist:completed", () => {
    let args = []; if (argv.path) {
      args.push(
        "--path", argv.path
      )
    }

    // --

    let electron = server.create()
    electron.start(args, () => {})

    // --

    gulp.watch(glob("app/*.{js,es6}").found, () => electron.restart(args))
    gulp.watch(glob("dist/*.{css,js}").found,
      electron.reload
    )
  })
})

And part of our dev.js:

import { client } from "electron-connect"

export default class Dev {
  constructor(env, BrowserWindow, mainWindow) {
    this.mainWindow = mainWindow
    this.BrowserWindow = BrowserWindow
    this.env = env
  }

  /*
   * Possibly enable the development tools.
   * @return {null}
   */
  possiblyEnable() {
    this.showChromeDefaultInspector()
    this.bindMainWindow()
  }

  /*
   * Displays Chromes inspector.
   * @return {null}
   */
  showChromeDefaultInspector() {
    this.mainWindow.toggleDevTools(
      //
    )
  }

  /*
   * Binds Electron-Connect to the mainWindow.
   * @return {null}
   */
  bindMainWindow() {
    if (this.env.nodeEnv == "development") {
      client.create(this.mainWindow)
    }

    return
  }
}

And our main stuff is pretty standard:

import Dev from "./dev"
import { default as electron, BrowserWindow } from "electron"
import Env from "./env"

// --

let mainWindow
const app = electron.app
const env = global.appEnv =
  new Env()

/*
 * Create the main application window.
 */
function createWindow () {
  mainWindow = new BrowserWindow({ width: 1280, height: 720 })
  mainWindow.loadURL(`file://${__dirname}/index.html`)
  mainWindow.on("closed", () => mainWindow = null)
  new Dev(env, BrowserWindow, mainWindow).
    possiblyEnable()
}

// --

app.on("ready", createWindow)
app.on("activate", () => {
  if (mainWindow === null) {
    createWindow()
  }
})

// --

app.on("window-all-closed", () => {
  if (process.platform !== "darwin") {
    app.quit()
  }
})

I also noticed that quite a few electron apps seem to be missing this very specific piece: mainWindow.on("closed", () => mainWindow = null) (which might make a difference? I don't know, I didn't bother to try.) We got that from: https://github.com/electron/electron-quick-start/blob/master/main.js#L22-L27 (Electrons own example.) That little snippet could be the difference between why it works for me and doesn't work for you.

from electron-connect.

@envygeeks your code shows that client.create() is called from browser (main) process. thats why it is working, as the issue is with client.create() from renderer process only.

True that mainWindow.on("closed", () => mainWindow = null) piece is missing in the examples. However, I always add that piece of code to the example, before testing and that doesn't help.

Probably, I should create a pull request updating the examples' code, to avoid any inconsistency.

from electron-connect.

I ran into this issue too and I found out this:

1. commenting these lines in client.js makes the electron process quit properly. https://github.com/Quramy/electron-connect/blob/master/lib/client.js#L87-L92
2. If we kill the electron process with this.electronProc.kill() instead of using the tree-kill package it works, but according to this commit 481dd3a there is a problem on windows with zombie child processes.

from electron-connect.

@karzac I think option 1 seems to be a better solution among them. Can you submit a pull request? I would like to see your fix merged, instead of #47.

from electron-connect.

@karzac @ssreekanth option 1 works fine. We also need to include

process.platform !== 'darwin'

here in this line https://github.com/Quramy/electron-connect/blob/master/lib/client.js#L86

from electron-connect.

@akashnimare why do you need that check there?

from electron-connect.

@ssreekanth it will show a error. OSX handles quit/closed event differently that's why we need to include that check. You can cross check it.

from electron-connect.

@akashnimare i'm not seeing any error. can you paste your console log here?

from electron-connect.

@ssreekanth this error -

Uncaught Exception:
A JavaScript error occurred in the main process -
Error: Attempting to call a function in a renderer window that has been closed or released. Function provided here: client.js:87:19.
    at BrowserWindow.callIntoRenderer (/Users/akka/dev/electronapps/electron-connect/node_modules/electron-prebuilt/dist/Electron.app/Contents/Resources/electron.asar/browser/rpc-server.js:189:19)
    at emitOne (events.js:96:13)
    at BrowserWindow.emit (events.js:188:7)

from electron-connect.

@akashnimare Is it complete stack trace from the log? If not, please paste the whole stack trace from the log..

from electron-connect.

@ssreekanth There are no errors in console log. Just above javascript warning only.

from electron-connect.

@akashnimare As i'm not able to reproduce your issue, would you mind to create a gist with sample code so that I can understand it better?

from electron-connect.

@ssreekanth wait let me make a gif.

from electron-connect.

And, in lib/client.js, what lines of code did you comment out exactly?

from electron-connect.

Commented out - https://github.com/Quramy/electron-connect/blob/master/lib/client.js#L87-L92
Updated - https://github.com/akashnimare/electron-connect/blob/master/lib/client.js#L86

from electron-connect.

Huh, I see your problem now. You should comment out these lines only - https://github.com/Quramy/electron-connect/blob/master/lib/client.js#L87-L91

No other change required. Try it out and let me know how it goes !!

from electron-connect.

@ssreekanth 👏 this also worked. Should I send a PR or we are waiting for some better solution?

from electron-connect.

Sure @akashnimare. PR would be great! This seems to be the best solution, at the moment.

from electron-connect.

@ssreekanth done here #51.

from electron-connect.

Closing this out with #51.

from electron-connect.