s.waitForBoot limit not implemented

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

s.waitForBoot limit not implemented

jamshark70-2
I just did a s.waitForBoot and boot failed (not SC's fault), which made me
wonder if the post-boot action would ever time out. So I looked and... the
bit that checks the limit is "not yet implemented" (3.9).

I guess there's a reason why (boot logic is inordinately complex), but it
led me to recompile the class library rather than risk something unexpected
when the server finally booted.

Just mentioning.

hjh

Sent with AquaMail for Android
http://www.aqua-mail.com




_______________________________________________
sc-dev mailing list

info (subscription, etc.): http://www.birmingham.ac.uk/facilities/ea-studios/research/supercollider/mailinglist.aspx
archive: http://www.listarc.bham.ac.uk/marchives/sc-dev/
search: http://www.listarc.bham.ac.uk/lists/sc-dev/search/
Reply | Threaded
Open this post in threaded view
|

Re: s.waitForBoot limit not implemented

julian.rohrhuber
Good you mention it. It’s kind of my responsibility because I did the refactoring. The reason is that I simply don’t understand how it should behave. There are a couple of parameters in the method that were apparently not used or at least opaque to me:

- mBootNotifyFirst (I don’t understand it)
- applicationRunning (weird semantics: serverRunning and: server.applicationRunning)

There are some fixes that Alberto de Campo proposed to improve the server booting procedure, but we don’t have this as a separate pull request yet.

If someone can explain to me how mBootNotifyFirst was originally meant to work, I’d try to fix it at an appropriate moment.




> On 19.05.2017, at 11:46, [hidden email] wrote:
>
> I just did a s.waitForBoot and boot failed (not SC's fault), which made me wonder if the post-boot action would ever time out. So I looked and... the bit that checks the limit is "not yet implemented" (3.9).
>
> I guess there's a reason why (boot logic is inordinately complex), but it led me to recompile the class library rather than risk something unexpected when the server finally booted.
>
> Just mentioning.
>
> hjh
>
> Sent with AquaMail for Android
> http://www.aqua-mail.com
>
>
>
>
> _______________________________________________
> sc-dev mailing list
>
> info (subscription, etc.): http://www.birmingham.ac.uk/facilities/ea-studios/research/supercollider/mailinglist.aspx
> archive: http://www.listarc.bham.ac.uk/marchives/sc-dev/
> search: http://www.listarc.bham.ac.uk/lists/sc-dev/search/


_______________________________________________
sc-dev mailing list

info (subscription, etc.): http://www.birmingham.ac.uk/facilities/ea-studios/research/supercollider/mailinglist.aspx
archive: http://www.listarc.bham.ac.uk/marchives/sc-dev/
search: http://www.listarc.bham.ac.uk/lists/sc-dev/search/
Reply | Threaded
Open this post in threaded view
|

Re: s.waitForBoot limit not implemented

jamshark70-2
 ---- On Fri, 19 May 2017 23:04:10 +0800 <[hidden email]> wrote ----
 > Good you mention it. It’s kind of my responsibility because I did the refactoring. The reason is that I simply don’t understand how it should behave. There are a couple of parameters in the method that were apparently not used or at least opaque to me:
 >  
 > - mBootNotifyFirst (I don’t understand it)
 > - applicationRunning (weird semantics: serverRunning and: server.applicationRunning)

AFAICS `bootNotifyFirst` was added here, but without comments or documentation.

https://github.com/supercollider/supercollider/commit/0c83a71789305195f7207213099bb3e9590c832c

(Editorial aside: "Object-oriented programming should be self-documenting" bwaaaah-hah-hah-hah-hah)

To check the behavior, I went back to 3.7. As near as I can figure out:

- `boot` sets `bootNotifyFirst = true` and immediately calls `doWhenBooted`
- `doWhenBooted` saves `bootNotifyFirst` in `mBootNotifyFirst` (method-local variable) and sets `bootNotifyFirst = false`.
- Those are the only places `bootNotifyFirst` is set, and there is no setter method. So `bootNotifyFirst` is always false, except when calling `doWhenBooted` from `boot`. So, there must be some desirable difference in behavior when the user calls `boot` vs `doWhenBooted`.
- `mBootNotifyFirst` is true if the user called `boot`.

```
while({
        ((serverRunning.not
          or: (serverBooting and: mBootNotifyFirst.not))
         and: {(limit = limit - 1) > 0})
        and: { pid.tryPerform(\pidRunning) == true }
}
```

Well, that's a mess... am I seriously the only SC user who writes comments when the logic gets complicated enough?

OK... deep breath... keep waiting as long as:

- Timeout limit has not been reached AND condition A
  - A = B or C
    - B: Server object hasn't yet recognized that it's running
    - C = D or E
      - D: Server object thinks it's in the process of booting
      - E: BUT ignore D if the user had called `boot`
- AND the currently-booting server process hasn't crashed

Or, maybe clearer to state the loop's stop condition:

- Stop if the server process died
- Or, stop if the timeout is reached (if `limit <= 0`, this "false" will nullify all of the ABCDE logic)
- Or, stop if the Server recognizes that it's running (`serverRunning` becomes true)
- Or, stop if `serverBooting` becomes false -- but only consider this if the user called `doWhenBooted` directly

How to untangle that into a spec... well, another time.

Maybe one part of the solution is to remove some of the ways to boot the server.

- boot
- waitForBoot
- doWhenBooted + user does something manually to boot the process (which I guess is really dodgy at this point, because then the user is responsible for connecting shared memory as well)
- Others?

I don't remember what was the rationale for the third one.

hjh


_______________________________________________
sc-dev mailing list

info (subscription, etc.): http://www.birmingham.ac.uk/facilities/ea-studios/research/supercollider/mailinglist.aspx
archive: http://www.listarc.bham.ac.uk/marchives/sc-dev/
search: http://www.listarc.bham.ac.uk/lists/sc-dev/search/
Reply | Threaded
Open this post in threaded view
|

Re: s.waitForBoot limit not implemented

jamshark70-2
 > Maybe one part of the solution is to remove some of the ways to boot the server.
 >  
 > - boot
 > - waitForBoot
 > - doWhenBooted + user does something manually to boot the process (which I guess is really dodgy at this point, because then the user is responsible for connecting shared memory as well)
 > - Others?

Or... ah-hah. An idea came to me while I was cleaning my electric shaver.

The code-design situation is that we have /n/ ways that a user could boot the server, and potentially /n/ conditions to stop waiting for the server to boot.

But the code structure is that we have one complicated condition that tries to distinguish between the ways that the user booted the server.

But if the code structure were a map between booting method and condition (which might just be a function), then it would be easier to read the code's purpose. And it would stop the dodgy practice of "we added a new way to boot the server, but it doesn't work with doWhenBooted, so let's hack the condition to make it sort-of work" because, if some user has new server-boot use case and an existing condition doesn't cover it, we can write a new condition tailored for the situation that includes only the required checks, no nonsense, no inscrutable boolean switches.

I'll leave it at that for now. I guess that might be a better way forward than trying to figure out how to re-hack the hacked-up hacks.

hjh


_______________________________________________
sc-dev mailing list

info (subscription, etc.): http://www.birmingham.ac.uk/facilities/ea-studios/research/supercollider/mailinglist.aspx
archive: http://www.listarc.bham.ac.uk/marchives/sc-dev/
search: http://www.listarc.bham.ac.uk/lists/sc-dev/search/