Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix 'Cannot set properties of undefined' #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mjmeintjes
Copy link

@mjmeintjes mjmeintjes commented Mar 8, 2025

Please review as I have no understanding of the code, but there was an obvious error in that the pr was undefined (it evaluated to cljs.core.pr, so hiding the problem), so I just replaced it with curr which seems to have fixed the error.

Fixes #77

pr evaluated to cljs.core.pr

TypeError: Cannot set properties of undefined (setting 'parent')
    at Object.missionary$impl$Ambiguous$ready [as ready] (eval at shadow$cljs$devtools$client$browser$global_eval (shadow.cljs.devtools.client.browser.js:1:1), <anonymous>:94:15)
    at G__261265.G__261265__0 [as cljs$core$IFn$_invoke$arity$0] (Ambiguous.cljs:330:28)
    at Object.missionary$impl$Eduction$pull [as pull] (Eduction.cljs:41:9)
    at missionary$impl$Eduction$transfer (Eduction.cljs:70:10)
    at Object.eval [as cljs$core$IDeref$_deref$arity$1] (Eduction.cljs:16:16)
    at Object.cljs$core$_deref [as _deref] (core.cljs:688:12)
    at Object.cljs$core$deref [as deref] (core.cljs:1477:4)
    at Object.missionary$impl$Ambiguous$backtrack [as backtrack] (Ambiguous.cljs:33:8)
    at missionary$impl$Ambiguous$branch (Ambiguous.cljs:73:8)
@mjmeintjes
Copy link
Author

After applying this change, the following code throws an exception when cancelling:

Running the following code in CLJS throws an exception while cancelling.

  (do
    (def input! (atom 10))
    (def task
      (ms/reduce
       (constantly nil)
       (ms/ap
        (let [n (or (ms/?< (ms/watch input!))
                    1)
              nxt (ms/?> (ms/seed (range n)))]
          (println nxt)))))
    (def cancel (task println #(js/console.error %)))
    (cancel))

Exception:

Cannot read properties of null (reading 'next')
    at Object.missionary$impl$Ambiguous$walk [as walk] (eval at shadow$cljs$devtools$client$browser$global_eval (shadow.cljs.devtools.client.browser.js:1:1), <anonymous>:5:58)
    at Object.missionary$impl$Ambiguous$ready [as ready] (Ambiguous.cljs:312:49)
    at G__261264.G__261264__0 [as cljs$core$IFn$_invoke$arity$0] (Ambiguous.cljs:323:28)
    at missionary$impl$Watch$kill (Watch.cljs:22:32)
    at Object.eval [as cljs$core$IFn$_invoke$arity$0] (Watch.cljs:7:20)
    at missionary$impl$Ambiguous$cancel (Ambiguous.cljs:92:9)
    at missionary$impl$Ambiguous$walk (eval at shadow$cljs$devtools$client$browser$global_eval (shadow.cljs.devtools.client.browser.js:1:1), <anonymous>:5:34)
    at missionary$impl$Ambiguous$kill (Ambiguous.cljs:85:6)

@leonoel
Copy link
Owner

leonoel commented Mar 10, 2025

You're right, it's a typo. pr was supposed to be an alias for curr so it's safe to just refer to curr.
Your fix is incomplete though, pr must be replaced with curr on L304-305 too. You can refer to the java implementation for reference

Processor pr = (Processor) curr;
Branch pivot = pr.child;
b.current = pivot;
pivot.parent = b;
while ((pr = pr.next) != curr) move(pivot, pr.child);
pr.prev = pr.next = pr;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot set properties of undefined (setting 'parent')
2 participants