Repository: nodejs/node
Pull request: diagnostics_channel: fix unsubscribe during publish

<!--
Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->

## Problem:
When unsubscribing a publisher that has already been published to, while inside a subscriber, the next subscriber will not be called. Example:
```javascript
const dc = require('diagnostics_channel')

const testChannel = dc.channel('test')

function subscriberA () {
  console.log('A called')
  testChannel.unsubscribe(subscriberA)
}

function subscriberB () {
  console.log('B called')
}

function subscriberC () {
  console.log('C called')
}

testChannel.subscribe(subscriberA)
testChannel.subscribe(subscriberB)
testChannel.subscribe(subscriberC)

testChannel.publish()
/*
Expected:
A called
B called
C called

Actual:
A called
C called
*/
```
This is due to the `publish()` method iterating over the array with incrementing index:
```javascript
  publish(data) {
    for (let i = 0; i < this._subscribers.length; i++) {
      const onMessage = this._subscribers[i];
      onMessage(data, this.name);
    }
  }
```
but `unsubscribe()` removing an element from that same array while it's being iterated upon:
```javascript
  unsubscribe(subscription) {
    const index = ArrayPrototypeIndexOf(this._subscribers, subscription);
    if (index === -1) return false;

    // !!! mutate the array and shift the indexes of the array making the loop skip a beat
    ArrayPrototypeSplice(this._subscribers, index, 1);

    return true;
  }
```

Thank you @iunanua for finding the bug!

## Solution:
~Instead of removing elements from array directly, set the element as undefined, and remove the undefined values later.~
New solution: just make shallow copies of the array instead of mutating it in subscribe and unsubscribe.

Changed files:
filename: lib/diagnostics_channel.js
patch: @@ -4,6 +4,8 @@ const {
   ArrayPrototypeAt,
   ArrayPrototypeIndexOf,
   ArrayPrototypePush,
+  ArrayPrototypePushApply,
+  ArrayPrototypeSlice,
   ArrayPrototypeSplice,
   ObjectDefineProperty,
   ObjectGetPrototypeOf,
@@ -97,6 +99,7 @@ function wrapStoreRun(store, data, next, transform = defaultTransform) {
 class ActiveChannel {
   subscribe(subscription) {
     validateFunction(subscription, 'subscription');
+    this._subscribers = ArrayPrototypeSlice(this._subscribers);
     ArrayPrototypePush(this._subscribers, subscription);
     channels.incRef(this.name);
   }
@@ -105,7 +108,10 @@ class ActiveChannel {
     const index = ArrayPrototypeIndexOf(this._subscribers, subscription);
     if (index === -1) return false;
 
-    ArrayPrototypeSplice(this._subscribers, index, 1);
+    const before = ArrayPrototypeSlice(this._subscribers, 0, index);
+    const after = ArrayPrototypeSlice(this._subscribers, index + 1);
+    this._subscribers = before;
+    ArrayPrototypePushApply(this._subscribers, after);
 
     channels.decRef(this.name);
     maybeMarkInactive(this);
@@ -137,9 +143,10 @@ class ActiveChannel {
   }
 
   publish(data) {
-    for (let i = 0; i < (this._subscribers?.length || 0); i++) {
+    const subscribers = this._subscribers;
+    for (let i = 0; i < (subscribers?.length || 0); i++) {
       try {
-        const onMessage = this._subscribers[i];
+        const onMessage = subscribers[i];
         onMessage(data, this.name);
       } catch (err) {
         process.nextTick(() => {

filename: test/parallel/test-diagnostics-channel-sync-unsubscribe.js
patch: @@ -9,6 +9,7 @@ const published_data = 'some message';
 const onMessageHandler = common.mustCall(() => dc.unsubscribe(channel_name, onMessageHandler));
 
 dc.subscribe(channel_name, onMessageHandler);
+dc.subscribe(channel_name, common.mustCall());
 
 // This must not throw.
 dc.channel(channel_name).publish(published_data);