diff --git a/README.md b/README.md index cd2386d..f77b86e 100644 --- a/README.md +++ b/README.md @@ -237,6 +237,8 @@ Error codes the daemon adds to mod-host's: | `-513` | `PHD_ERR_NO_NOTE_INPUT` | the instance's plugin takes no notes (an effect): its worker answers note_on, note_off and all_notes_off with this code | | `-514` | `PHD_ERR_NOTES_BY_JACK` | the instance's worker takes no notes on its socket (mod-host): the daemon answers the note verbs and note_input with this code, and the plugin is played through its jack MIDI input with connect | | `-515` | `PHD_ERR_NO_PRESET_LIST` | the instance's worker cannot list the plugin's presets (mod-host): the daemon answers preset_list and preset_get with this code; preset_load still takes a preset the controller knows | +| `-516` | `PHD_ERR_PIN_REFUSED` | the plugin's pin carries verdict=refused: its files are the ones that failed qualification, so add is refused and no worker sees it, and a replayed add is refused the same way; files that differ from the pin answer (-509) first | +| `-517` | `PHD_ERR_NOTE_QUEUE_FULL` | the instance's worker has no room for this note_on, note_off or all_notes_off before its next block: the line was not taken and nothing of it is kept; the controller may send it again | A worker's own refusal is returned as it said it. An `add` that killed its worker answers mod-host's `-102`. @@ -362,7 +364,10 @@ files, keeps the pin in the pin store (`$XDG_STATE_HOME/plugin-hostd/pins`) and asked of the worker after the add (`pin_layout`) and recorded too. A host that pins by itself sets `pin_mode strict`. The store, a `pin_file` and `pin_set` carry one line format, `pin_set [verdict=]` with the verb optional in a file. In `first-use` only a pin with `verdict=in-process`, which plugin-qualify writes, lets a plugin -share a worker. A plugin that changed since it was pinned is refused (-509, or -510 from the worker), the +share a worker. A pin with `verdict=refused` names files that failed qualification. Their `add` answers `-516` +(`PIN_REFUSED`), no worker sees it, and a replay is refused with the same code. A pin that changes to `refused` while +the instance runs takes effect at its next replay. Files that differ from the pin answer `-509` first, because the +verdict was given for the pinned files only. A plugin that changed since it was pinned is refused (-509, or -510 from the worker), the `pin_changed files|layout` event names what changed, and `pin_accept ` takes the new pin. What cannot be pinned: diff --git a/include/plugin-hostd/protocol.h b/include/plugin-hostd/protocol.h index 33f2f37..e86b401 100644 --- a/include/plugin-hostd/protocol.h +++ b/include/plugin-hostd/protocol.h @@ -260,6 +260,8 @@ static inline int phd_pool_name_valid(const char *name) #define PHD_ERR_NO_NOTE_INPUT (-513) #define PHD_ERR_NOTES_BY_JACK (-514) #define PHD_ERR_NO_PRESET_LIST (-515) +#define PHD_ERR_PIN_REFUSED (-516) +#define PHD_ERR_NOTE_QUEUE_FULL (-517) /* X(id, meaning) */ #define PHD_ERRORS(X) \ @@ -294,7 +296,13 @@ static inline int phd_pool_name_valid(const char *name) "with connect") \ X(NO_PRESET_LIST, "the instance's worker cannot list the plugin's presets (mod-host): the daemon answers " \ PHD_VERB_PRESET_LIST " and " PHD_VERB_PRESET_GET " with this code; preset_load still takes a preset " \ - "the controller knows") + "the controller knows") \ + X(PIN_REFUSED, "the plugin's pin carries " PHD_PIN_VERDICT_KEY PHD_PIN_VERDICT_REFUSED ": its files are the ones that " \ + "failed qualification, so add is refused and no worker sees it, and a replayed add is refused the " \ + "same way; files that differ from the pin answer " PHD_STR(PHD_ERR_PIN_BINARY_MISMATCH) " first") \ + X(NOTE_QUEUE_FULL, "the instance's worker has no room for this " PHD_VERB_NOTE_ON ", " PHD_VERB_NOTE_OFF " or " \ + PHD_VERB_ALL_NOTES_OFF " before its next block: the line was not taken and nothing of it is kept; the " \ + "controller may send it again") /* ---------------------------------------------------------------- verbs */ diff --git a/protocol/plugin-hostd.json b/protocol/plugin-hostd.json index fabe69b..e53434f 100644 --- a/protocol/plugin-hostd.json +++ b/protocol/plugin-hostd.json @@ -38,7 +38,9 @@ {"name": "LV2_ADAPTER_REFUSED", "macro": "PHD_ERR_LV2_ADAPTER_REFUSED", "code": -512, "meaning": "the lv2-clap worker's LV2 adapter cannot run this plugin (MIDI it does not feed, a required feature it lacks, a layout it does not take): the worker answered the add -102, the lv2_adapter_refused event carries the hosting.* code of that reply, and under lv2_worker auto the plugin is run by the lv2 row instead and this code is never answered; a plugin no bundle of the worker's world holds is no refusal: the worker's -101 is answered"}, {"name": "NO_NOTE_INPUT", "macro": "PHD_ERR_NO_NOTE_INPUT", "code": -513, "meaning": "the instance's plugin takes no notes (an effect): its worker answers note_on, note_off and all_notes_off with this code"}, {"name": "NOTES_BY_JACK", "macro": "PHD_ERR_NOTES_BY_JACK", "code": -514, "meaning": "the instance's worker takes no notes on its socket (mod-host): the daemon answers the note verbs and note_input with this code, and the plugin is played through its jack MIDI input with connect"}, - {"name": "NO_PRESET_LIST", "macro": "PHD_ERR_NO_PRESET_LIST", "code": -515, "meaning": "the instance's worker cannot list the plugin's presets (mod-host): the daemon answers preset_list and preset_get with this code; preset_load still takes a preset the controller knows"} + {"name": "NO_PRESET_LIST", "macro": "PHD_ERR_NO_PRESET_LIST", "code": -515, "meaning": "the instance's worker cannot list the plugin's presets (mod-host): the daemon answers preset_list and preset_get with this code; preset_load still takes a preset the controller knows"}, + {"name": "PIN_REFUSED", "macro": "PHD_ERR_PIN_REFUSED", "code": -516, "meaning": "the plugin's pin carries verdict=refused: its files are the ones that failed qualification, so add is refused and no worker sees it, and a replayed add is refused the same way; files that differ from the pin answer (-509) first"}, + {"name": "NOTE_QUEUE_FULL", "macro": "PHD_ERR_NOTE_QUEUE_FULL", "code": -517, "meaning": "the instance's worker has no room for this note_on, note_off or all_notes_off before its next block: the line was not taken and nothing of it is kept; the controller may send it again"} ], "verbs": [ {"name": "add", "arguments": " [own | pool: | default] [client_name]", "reply": "resp ", "meaning": "mod-host's add with a placement; a token after the instance that is not a placement is the jack client name"}, diff --git a/src/pins.c b/src/pins.c index d3484db..f1b3583 100644 --- a/src/pins.c +++ b/src/pins.c @@ -663,6 +663,12 @@ int pins_check(const format_t *format, const char *uri, const char *search, cons names_free(&files); for (n = 0; n < p->nfiles && rc == SUCCESS; n++) rc = hash_file(root, p->path[n], p->digest[n], uri); + /* the verdict is about these very files: only once they are the pinned ones does a refused one refuse the add */ + if (rc == SUCCESS && !strcmp(p->verdict, PHD_PIN_VERDICT_REFUSED)) + { + fprintf(stderr, "plugin-hostd: %s: its pin carries " PHD_PIN_VERDICT_KEY PHD_PIN_VERDICT_REFUSED "\n", uri); + rc = PHD_ERR_PIN_REFUSED; + } if (rc == SUCCESS) *layout = strcmp(p->layout, PHD_PIN_LAYOUT_UNKNOWN) ? p->layout : NULL; return rc; diff --git a/src/pins.h b/src/pins.h index 65027df..53ff752 100644 --- a/src/pins.h +++ b/src/pins.h @@ -58,7 +58,8 @@ int pins_set_layout(const char *uri, const char *layout); * loads for it (format->pin_collect, searched in `search`) all held by the pin, then every file the pin names hashed * over one open descriptor. SUCCESS, with *layout the * layout pin ":" (the table's, valid until the next pin verb) or NULL while the pin's layout is - * unknown, or PHD_ERR_PIN_ABSENT or PHD_ERR_PIN_BINARY_MISMATCH. */ + * unknown, or PHD_ERR_PIN_ABSENT or PHD_ERR_PIN_BINARY_MISMATCH, or PHD_ERR_PIN_REFUSED for the pinned files under a + * verdict of refused. */ int pins_check(const format_t *format, const char *uri, const char *search, const char **layout); /* a refusal of a pin, with the reason on stderr: PHD_ERR_PIN_BINARY_MISMATCH */ diff --git a/tests/consumer.c b/tests/consumer.c index f4bd2a1..7d1f47c 100644 --- a/tests/consumer.c +++ b/tests/consumer.c @@ -26,6 +26,26 @@ int main(void) #define X(id, meaning) errors++; check(PHD_ERR_##id < 0, #id " is a refusal"); PHD_ERRORS(X) #undef X + /* each code is the supervisor's own, in its hundred, and no two rows share one: a reply names one refusal */ + { + static const int codes[] = { +#define X(id, meaning) PHD_ERR_##id, + PHD_ERRORS(X) +#undef X + }; + size_t a, b; + + for (a = 0; a < sizeof(codes) / sizeof(codes[0]); a++) + { + check(codes[a] <= -501 && codes[a] >= -599, "every code is in the -5xx hundred"); + for (b = a + 1; b < sizeof(codes) / sizeof(codes[0]); b++) + if (codes[a] == codes[b]) + { + printf("FAIL two errors share the code %d\n", codes[a]); + failures++; + } + } + } #define X(id, name, args, rep, meaning) verbs++; check(*name && *rep, #id " is named and answers"); PHD_VERBS(X) #undef X diff --git a/tests/daemon_test.py b/tests/daemon_test.py index 3c425dc..4bd1f9c 100644 --- a/tests/daemon_test.py +++ b/tests/daemon_test.py @@ -1016,6 +1016,23 @@ def notes_reach_a_clap_worker_unchanged_and_an_effect_refuses_them(): os.unlink(log) +@test +def a_worker_whose_note_queue_is_full_answers_the_declared_code_and_the_daemon_passes_it(): + """omx-clap-host refuses a note it has no room for in the block with NOTE_QUEUE_FULL; the daemon hands the code to the + controller unchanged and keeps nothing of the note""" + d = daemon() + try: + full = resp("NOTE_QUEUE_FULL") + check(full != resp("NO_NOTE_INPUT") and int(full.split()[1]) <= -501, "a code of its own: %s" % full) + d.expect("add clap:fake#synth_full 0", "resp 0") + d.expect("note_on 0 0 60 100", full) + d.expect("note_off 0 0 60 0", full) + d.expect("all_notes_off 0", full) + d.expect("note_input 0", "resp 0 midi_in") + finally: + d.close() + + @test def notes_are_never_replayed(): log = tempfile.mktemp(prefix="plugin-hostd-fake-log.") @@ -1464,6 +1481,51 @@ def a_layout_pinned_again_while_its_worker_was_down_is_refused_by_the_replayed_a p.close() +@test +def a_pin_whose_verdict_is_refused_refuses_the_add_with_its_own_code(): + """issue 71: verdict=refused names bytes that failed qualification; an add of them is refused by code, no worker + sees it, and a replay of an instance whose pin turned refused is refused the same way""" + p = Plugins() + d = p.daemon() + refused = harness.CODE.get("PIN_REFUSED") + try: + check(refused is not None, "the protocol declares PIN_REFUSED: %s" % sorted(harness.CODE)) + d.expect(p.clap_pin() + " verdict=refused", "resp 0") + r = d.send("add %s 0" % p.clap_uri) + check(r == "resp %d" % refused, "the add of a refused plugin answers PIN_REFUSED: %r" % r) + check(d.workers() == {} and p.received() == [], "no worker started, none was told: %s %s" % (d.workers(), p.received())) + # positive control: the same bytes qualified again run + d.expect(p.clap_pin() + " verdict=isolated", "resp 0") + d.expect("add %s 0" % p.clap_uri, "resp 0") + # a pin turned refused while the instance runs: its replay is refused with the same code + _, w = d.holder(0) + d.expect(p.clap_pin() + " verdict=refused", "resp 0") + mark = d.mark() + os.kill(w["pid"], signal.SIGKILL) + d.wait_event("instance_replay_refused 0 %d" % refused, since=mark) + check(not [e for e in d.events[mark:] if e.startswith("pin_changed")], "a refusal is no change of the files") + finally: + d.close() + # bytes that changed are a mismatch first: a verdict is about the bytes it was given for + d = p.daemon() + try: + d.expect(p.clap_pin().replace(sha(p.clap)[:8], "00000000") + " verdict=refused", "resp 0") + d.expect("add %s 1" % p.clap_uri, resp("PIN_BINARY_MISMATCH")) + finally: + d.close() + # a pin_file carries the verdict as pin_set does + pins = os.path.join(p.dir, "pins.txt") + write(pins, ("%s verdict=refused\n" % p.clap_pin().split(" ", 1)[1]).encode()) + d = p.daemon(conf={"pin_mode": "first-use", "pin_file": pins}) + try: + r = d.send("add %s 2" % p.clap_uri) + check(r == "resp %d" % refused, "a pin_file's refused verdict refuses the add: %r" % r) + check(store_lines(p) == [], "and the refused plugin is not learned into the store") + finally: + d.close() + p.close() + + def own_worker_of(d, inst): return d.holder(inst)[1]["place"] == "own" diff --git a/tests/fake_host.c b/tests/fake_host.c index ce5f181..d878488 100644 --- a/tests/fake_host.c +++ b/tests/fake_host.c @@ -52,7 +52,8 @@ * preset_list "resp 2", and preset_get <0|1> one preset each, -902 for any other index * track_info ... "resp 0"; the fake keeps nothing of it (FAKE_LOG shows what it was sent) * note_input "resp 0 midi_in" for an instance whose uri holds "synth", "resp 0 -" for any other - * note_on, note_off, all_notes_off ... "resp 0" for a synth, PHD_ERR_NO_NOTE_INPUT for any other; the fake + * note_on, note_off, all_notes_off ... "resp 0" for a synth, PHD_ERR_NOTE_QUEUE_FULL for one whose uri holds + * "synth_full", PHD_ERR_NO_NOTE_INPUT for any other; the fake * plays nothing (FAKE_LOG shows what it was sent) * set_bpm, set_bpb, transport, transport_sync "resp 0", as mod-host answers them, -902 with FAKE_REFUSE_SETTINGS, as * omx-clap-host answers them; the fake keeps nothing of them @@ -101,6 +102,7 @@ typedef struct INST_T { int exists; int nostate; int synth; + int full; /* a synth whose note queue is always full */ char client[64]; char name[MAX_PARAMS][64]; char text[MAX_PARAMS][128]; @@ -310,6 +312,7 @@ static void receive(msg_t *msg) g_expect[id][0] = '\0'; g_inst[id].nostate = strstr(tok[1], "nostate") != NULL; g_inst[id].synth = strstr(tok[1], "synth") != NULL; + g_inst[id].full = strstr(tok[1], "synth_full") != NULL; snprintf(g_inst[id].client, sizeof(g_inst[id].client), "%s", ntok > 3 ? tok[3] : ""); answer(fd, "resp %d", id); } @@ -449,7 +452,7 @@ static void receive(msg_t *msg) else if (!strcmp(verb, PHD_VERB_NOTE_INPUT) && ntok == 2) answer(fd, "resp 0 %s", g_inst[n].synth ? "midi_in" : PHD_NO_NOTE_INPUT); else if (!strcmp(verb, PHD_VERB_NOTE_ON) || !strcmp(verb, PHD_VERB_NOTE_OFF) || !strcmp(verb, PHD_VERB_ALL_NOTES_OFF)) - answer(fd, "resp %d", g_inst[n].synth ? 0 : PHD_ERR_NO_NOTE_INPUT); + answer(fd, "resp %d", !g_inst[n].synth ? PHD_ERR_NO_NOTE_INPUT : g_inst[n].full ? PHD_ERR_NOTE_QUEUE_FULL : 0); else if (!strcmp(verb, PHD_VERB_REMOTE_PAGES) && ntok == 2) answer(fd, "resp 2"); else if (!strcmp(verb, PHD_VERB_REMOTE_PAGE_GET) && ntok == 3 && !strcmp(tok[2], "0")) diff --git a/tests/sabotage.py b/tests/sabotage.py index 8bb7529..e370622 100644 --- a/tests/sabotage.py +++ b/tests/sabotage.py @@ -13,7 +13,7 @@ # an LV2 bundle (MOD_HOST, LV2_DIR, LV2_URI, LV2_BUNDLE, LV2_PARAM) and is skipped without them; "pin" breaks # include/plugin-hostd/pin.h and runs tests/pin_test.c; "readme" breaks README.md or BUILDING.md and runs # tests/readme_build.py, with no daemon to build; "probeschema" breaks protocol/probe-output.schema.json and runs -# tests/probe_schema.py; "probe" breaks src/probe.c and runs tests/probe_test.py; "nopins" breaks the packaging or a workflow and runs tests/no_pins.py +# tests/probe_schema.py; "consumer" breaks the header and compiles tests/consumer.c against it; "probe" breaks src/probe.c and runs tests/probe_test.py; "nopins" breaks the packaging or a workflow and runs tests/no_pins.py SABOTAGE = [ ("the verb tail is not replayed", "src/supervisor.c", "if (replay_line(w, i->tail[m].line) != RPC_OK)", "if (0 && replay_line(w, i->tail[m].line) != RPC_OK)", @@ -266,6 +266,11 @@ ("an add the worker died on is not named by an event", "src/supervisor.c", " if (code != PHD_ERR_VERB_DROPPED)\n event(PHD_EVENT_VERB_DROPPED_FMT, id, line);\n", "", "under_auto_a_refused_plugin_that_kills_mod_host_on_add_costs_only_its_own_worker"), + ("a pin whose verdict is refused admits the add", "src/pins.c", + " if (rc == SUCCESS && !strcmp(p->verdict, PHD_PIN_VERDICT_REFUSED))\n", " if (0)\n", + "a_pin_whose_verdict_is_refused_refuses_the_add_with_its_own_code"), + ("two errors share a code", "include/plugin-hostd/protocol.h", "#define PHD_ERR_NOTE_QUEUE_FULL (-517)", + "#define PHD_ERR_NOTE_QUEUE_FULL (-516)", "consumer", "consumer"), ("the build steps are back on the README", "README.md", "Building from source: see [BUILDING.md](BUILDING.md).", "Building from source:\n\n make\n\nsee [BUILDING.md](BUILDING.md).", "README.md runs no build tool", "readme"), ("BUILDING.md lost its build command", "BUILDING.md", " make [MOD_HOST_DIR=]\n", "", @@ -417,7 +422,18 @@ def run_format_test(tree): return p.returncode, p.stdout + ("ok format_test\n" if p.returncode == 0 else "") +def run_consumer_test(tree): + """tests/consumer.c against the tree's header, as a consumer compiles it; "ok consumer" when it passes""" + exe = os.path.join(tree, "consumer") + subprocess.run(["cc", "-I" + os.path.join(tree, "include"), "-std=c99", "-Wall", "-Wextra", "-o", exe, + os.path.join(ROOT, "tests", "consumer.c")], check=True) + p = subprocess.run([exe], capture_output=True, text=True, timeout=60) + return p.returncode, p.stdout + ("ok consumer\n" if p.returncode == 0 else "") + + def run_test(tree, name, jack=False): + if jack == "consumer": + return run_consumer_test(tree) if jack == "format": return run_format_test(tree) if jack == "pin": @@ -477,6 +493,8 @@ def run_test(tree, name, jack=False): elif jack == "probeschema": os.makedirs(os.path.join(tree, "protocol")) shutil.copy(os.path.join(ROOT, "protocol", "probe-output.schema.json"), os.path.join(tree, "protocol")) + elif jack == "consumer": + shutil.copytree(os.path.join(ROOT, "include"), os.path.join(tree, "include")) elif jack == "nopins": for f in ("packaging/plugin-hostd.spec", "debian/control", ".github/workflows/ci.yml"): os.makedirs(os.path.dirname(os.path.join(tree, f)), exist_ok=True) @@ -499,7 +517,7 @@ def run_test(tree, name, jack=False): open(os.path.join(tree, path), "w").write(src.replace(text, repl)) if jack == "probe": build_probe(tree) - elif jack not in ("readme", "nopins", "format", "probeschema"): + elif jack not in ("readme", "nopins", "format", "consumer", "probeschema"): for f in os.listdir(os.path.join(tree, "src")): if f.endswith(".o"): os.unlink(os.path.join(tree, "src", f))