Using files to store pid and refactor processes termination #2

vderunov · 2024-02-19T18:48:29Z

No description provided.

noisekit · 2024-02-20T08:47:48Z

src/main/ipfs.ts

+    if (pid) {
+      await removePidFile();
+      logger.log('Shutting down the IPFS daemon');
+      await rpcRequest('shutdown');


execSync(process.platform === 'win32' ? 'ipfs.exe shutdown' : '.synthetix/go-ipfs/ipfs shutdown')

noisekit · 2024-02-20T15:49:39Z

src/main/follower.ts

-    // whatever
+    const pid = readFileSync(PID_FOLLOWER_FILE_PATH, 'utf8');
+    if (pid) {
+      rmSync(PID_FOLLOWER_FILE_PATH);


Suggested change

rmSync(PID_FOLLOWER_FILE_PATH);

rmSync(path.join(ROOT, 'ipfs-cluster-follow.pid'), {force: true});

PID_FOLLOWER_FILE_PATH removed

noisekit · 2024-02-20T15:49:53Z

src/main/follower.ts

+      if (process.platform === 'win32') {
+        execSync(`taskkill /F /PID ${pid}`);
+      } else {
+        process.kill(Number(pid));
+      }


process.kill as native nodejs method should be cross-platform

Suggested change

if (process.platform === 'win32') {

execSync(`taskkill /F /PID ${pid}`);

} else {

process.kill(Number(pid));

}

process.kill(Number(pid));

You are right when I caught some error and thought that process.kill() only was the Unix system. I was wrong.

noisekit · 2024-02-20T15:51:34Z

src/main/follower.ts

+      logger.log(`follower kill: PID ${pid} killed and PID file removed`);
+    }
+  } catch (e) {
+    logger.log('follower kill error:', e);


logger.error(e)

But logging is not working during shutdown so probably not much sense to have it at all

noisekit · 2024-02-20T15:52:55Z

src/main/follower.ts

  } catch (_error) {
+    logger.log(`followerId: `, _error);
    return '';


Suggested change

} catch (_error) {

logger.log(`followerId: `, _error);

return '';

} catch (error) {

logger.error(error);

return '';

Guess these are just debugging things for draft anyway

exactly, removed

noisekit · 2024-02-20T15:55:24Z

src/main/ipfs.ts

-    rmSync(path.join(IPFS_PATH, 'repo.lock'), { recursive: true });
-  } catch (_e) {
-    // whatever
+    if (readFileSync(PID_IPFS_FILE_PATH, 'utf8')) {


no need for if. just do rmSync(PID_IPFS_FILE_PATH, {force: true}); - it will not fail even if file does not exist.
Also only delete PID and repo.lock after IPFS was shut down.

removed if()

removed PID_IPFS_FILE_PATH const

moved file deletion after the shutdown. BUT, I moved them up for a reason, because the files were not deleted when I interrupted the execution of the program (server) with the keyboard Ctrl+C. I don't know if this case is important for us or not?

Sounds like node process does not wait for ipfs to fully shutdown? I can see you moved rm after shutdown - did it work this time?
I reckon we cannot remove ipfs locks until ipfs if fully off anyway. We could do that with PID.
If there are issues - the order can be

remove pid

shutdown ipfs

remove locks

noisekit · 2024-02-20T15:57:16Z

src/main/main.ts

 ipcMain.handle('follower-isInstalled', followerIsInstalled);
 ipcMain.handle('ipfs-isRunning', ipfsIsRunning);
-ipcMain.handle('follower-isRunning', followerPid);
+ipcMain.handle('follower-isRunning', () => getPid(PID_FOLLOWER_FILE_PATH));


Suggested change

ipcMain.handle('follower-isRunning', () => getPid(PID_FOLLOWER_FILE_PATH));

ipcMain.handle('follower-isRunning', () => fs.readFile(path.join(ROOT, 'ipfs-cluster-follow.pid'), 'utf8').catch(() => null));

removed getPid()

noisekit · 2024-02-20T15:59:02Z

src/main/follower.ts

-      ? 'ipfs-cluster-follow.exe'
-      : '.synthetix/ipfs-cluster-follow/ipfs-cluster-follow synthetix run'
-  );
+  const pid = await getPid(PID_FOLLOWER_FILE_PATH);


Suggested change

const pid = await getPid(PID_FOLLOWER_FILE_PATH);

const pid = await fs.readFile(path.join(ROOT, 'ipfs-cluster-follow.pid'), 'utf8').catch(() => null);

removed pid.ts

noisekit

Looking great

* Using files to store pid and refactor processes termination (#2) * Using files to store pid and refactor processes termination * Refactor process termination * Update logging message during IPFS daemon termination * Refactor process termination and returned getPid async * Deleting a file before processes are terminated * Fix review comments * Add to fs.readFile to catch errors handler * Transition to Electron Forge * Replace npm with yarn in config.yml * Add dpkg and fakeroot dependencies in CircleCI config * Add rpm to CircleCI configuration * Convert PNG icon into a true ICO icon * Linux support * Remove compiled code * Enable global yarn cache * Lint script * Lint harder * Fix `make` in CCI * Renames * Update building and packaging * Update deps * Fix for empty args * Support ipns codec * No need to tear down * Return config * Return icons * Add back svg and config checks * Update readme for config pinning * Fix teardown * Return svg icons * Remove node-fetch * Lint harder * Fix typo * Package not make --------- Co-authored-by: Noisekit <nikita@cc.snxdao.io>

* Using files to store pid and refactor processes termination (#2) * Using files to store pid and refactor processes termination * Refactor process termination * Update logging message during IPFS daemon termination * Refactor process termination and returned getPid async * Deleting a file before processes are terminated * Fix review comments * Add to fs.readFile to catch errors handler * Fix dock icon on macOS * Add custom app icons configuration and DPI support * Update icon paths in forge config and icons generator * Remove hardcoded paths to app icons * Remove generation icon.ico from icongen.js * Run app.dock.setIcon() only for development mode * Add app icon for develop mode * Replace __dirname with app.getAppPath()

* Using files to store pid and refactor processes termination (#2) * Using files to store pid and refactor processes termination * Refactor process termination * Update logging message during IPFS daemon termination * Refactor process termination and returned getPid async * Deleting a file before processes are terminated * Fix review comments * Add to fs.readFile to catch errors handler * Add authentication flow - first commit * Add authentication flow - first commit * Fix signMessage * Add if condition for signedMessage * Add token management and UI signing feedback * Add application mutation hooks and AccessControl component * Remove unused icon URL and adjust footer alignment * Add FormControl for input validation * Add localhost API URL and success callbacks * Refactor fetch logic to a reusable function * Remove unused approval/rejection logic * Refactor headers * Remove boxShadow * Add environment variable support with dotenv * Add React Router for navigation * Add RPC URL * Add NetworkMismatch component * Remove unused URL and adjust main window height * Apply suggestions from code review --------- Co-authored-by: Noisekit <28145325+noisekit@users.noreply.github.com>

vderunov added 3 commits February 19, 2024 20:47

Using files to store pid and refactor processes termination

cbb3cc7

Refactor process termination

de18148

Update logging message during IPFS daemon termination

8c38b4d

noisekit reviewed Feb 20, 2024

View reviewed changes

vderunov added 2 commits February 20, 2024 15:41

Refactor process termination and returned getPid async

ae54aab

Deleting a file before processes are terminated

1c4e2ce

noisekit reviewed Feb 20, 2024

View reviewed changes

Fix review comments

c5b22fe

vderunov marked this pull request as ready for review February 20, 2024 21:44

Add to fs.readFile to catch errors handler

c683a61

noisekit approved these changes Feb 21, 2024

View reviewed changes

vderunov merged commit 5456f6b into master Feb 21, 2024

noisekit deleted the improve-pid-handling branch February 22, 2024 13:54

	rmSync(PID_FOLLOWER_FILE_PATH);
	rmSync(path.join(ROOT, 'ipfs-cluster-follow.pid'), {force: true});

	ipcMain.handle('follower-isRunning', () => getPid(PID_FOLLOWER_FILE_PATH));
	ipcMain.handle('follower-isRunning', () => fs.readFile(path.join(ROOT, 'ipfs-cluster-follow.pid'), 'utf8').catch(() => null));

	const pid = await getPid(PID_FOLLOWER_FILE_PATH);
	const pid = await fs.readFile(path.join(ROOT, 'ipfs-cluster-follow.pid'), 'utf8').catch(() => null);

Using files to store pid and refactor processes termination #2

Using files to store pid and refactor processes termination #2

Uh oh!

Conversation

vderunov commented Feb 19, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noisekit Feb 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noisekit Feb 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noisekit left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

noisekit Feb 20, 2024 •

edited

Loading

noisekit Feb 20, 2024 •

edited

Loading