[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15669747#comment-15669747
 ] 

ASF GitHub Bot commented on ZOOKEEPER-761:
------------------------------------------

Github user rgs1 commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/90#discussion_r88183747
  
    --- Diff: src/c/src/zookeeper.c ---
    @@ -4052,6 +3958,76 @@ int zoo_amulti(zhandle_t *zh, int count, const 
zoo_op_t *ops,
         return (rc < 0) ? ZMARSHALLINGERROR : ZOK;
     }
     
    +
    +int zoo_aremove_watchers(zhandle_t *zh, const char *path, ZooWatcherType 
wtype,
    +        watcher_fn watcher, void *watcherCtx, int local,
    +        void_completion_t *completion, const void *data)
    +{
    +    char *server_path = prepend_string(zh, path);
    +    int rc;
    +    struct oarchive *oa;
    +    struct RequestHeader h = { get_xid(), ZOO_REMOVE_WATCHES };
    +    struct RemoveWatchesRequest req =  { (char*)server_path, wtype };
    +    watcher_deregistration_t *wdo;
    +
    +    if (!zh || !isValidPath(server_path, 0)) {
    +        rc = ZBADARGUMENTS;
    +        goto done;
    +    }
    +
    +    if (!local && is_unrecoverable(zh)) {
    +        rc = ZINVALIDSTATE;
    +        goto done;
    +    }
    +
    +    if (!pathHasWatcher(zh, server_path, wtype, watcher, watcherCtx)) {
    +        rc = ZNOWATCHER;
    +        goto done;
    +    }
    +
    +    if (local) {
    +        removeWatchers(zh, server_path, wtype, watcher, watcherCtx);
    +#ifdef THREADED
    +        notify_sync_completion((struct sync_completion *)data);
    --- End diff --
    
    @breed @fpj btw -- sorry for the confusing code. `zoo_aremove_watchers` is 
sui generis given that it's the only public method that can return `ZOK` 
without scheduling a remote call (for which then, the callback would be 
naturally dispatched). Thus, this horrible hack of calling 
`notify_sync_completion()`. 


> Remove *synchronous* calls from the *single-threaded* C clieant API, since 
> they are documented not to work
> ----------------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-761
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-761
>             Project: ZooKeeper
>          Issue Type: Improvement
>          Components: c client
>    Affects Versions: 3.1.1, 3.2.2
>         Environment: RHEL 4u8 (Linux).  The issue is not OS-specific though.
>            Reporter: Jozef Hatala
>            Assignee: Benjamin Reed
>            Priority: Minor
>             Fix For: 3.5.3, 3.6.0
>
>         Attachments: fix-sync-apis-in-st-adaptor.patch, 
> fix-sync-apis-in-st-adaptor.v2.patch
>
>
> Since the synchronous calls are 
> [known|http://hadoop.apache.org/zookeeper/docs/current/zookeeperProgrammers.html#Using+the+C+Client]
>  to be unimplemented in the single threaded version of the client library 
> libzookeeper_st.so, I believe that it would be helpful towards users of the 
> library if that information was also obvious from the header file.
> Anecdotally more than one of us here made the mistake of starting by using 
> the synchronous calls with the single-threaded library, and we found 
> ourselves debugging it.  An early warning would have been greatly appreciated.
> 1. Could you please add warnings to the doxygen blocks of all synchronous 
> calls saying that they are not available in the single-threaded API.  This 
> cannot be safely done with {{#ifdef THREADED}}, obviously, because the same 
> header file is included whichever client library implementation one is 
> compiling for.
> 2. Could you please bracket the implementation of all synchronous calls in 
> zookeeper.c with {{#ifdef THREADED}} and {{#endif}}, so that those symbols 
> are not present in libzookeeper_st.so?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to