stop async watch, add new watcher and restart, the watcher is invalid

[xiaoyulei]

the example code is like this below. I add a watcher and start async watch. Then stop it, add another watcher and start it again. I found that only the second watcher can receive watch event, the first is invalid.

cetcd_client m_cli;
cetcd_array m_addrs;
cetcd_array m_watchers;

cetcd_client_init(&m_cli, &m_addrs);
cetcd_array_init(&m_watchers, 1);
cetcd_watcher* watcher = cetcd_watcher_create(&m_cli, getKey1(), 0, 1 , 0, handleEvent1, NULL);
cetcd_add_watcher(&m_watchers, watcher);
cetcd_multi_watch_async(&m_cli, &m_watchers);
cetcd_multi_watch_async_stop(&m_cli, m_wid);

cetcd_watcher* watcher = cetcd_watcher_create(&m_cli, getKey2(), 0, 1 , 0, handleEvent2, NULL);
cetcd_add_watcher(&m_watchers, watcher);
cetcd_multi_watch_async(&m_cli, &m_watchers);

[xiaoyulei]

@shafreeck @Huang-lin

[shafreeck]

There is some problem with your code above

First

cetcd_watcher* watcher = cetcd_watcher_create(&m_cli, getKey1(), 0, 1 , 0, handleEvent1, NULL);
cetcd_multi_watch_async(&m_cli, &m_watchers);

The watcher should be add to m_watchers first using cetcd_add_watcher

Second

cetcd_watcher* watcher = cetcd_watcher_create(&m_cli, getKey2(), 0, 1 , 0, handleEvent2, NULL);
cetcd_multi_watch_async(&m_cli, &m_watchers);

This is the same as above case, add it before watching.

Finally

You may have added the watcher in you real code. The answer to your question is that it is a feature not a bug.

All watchers will be recycled when stop watching, so you can not use it later. We can not support adding watcher dynamic when watching has started. This is really a limitation. You can work around this using your code above with tiny fixes.

The fix is that you do not have to stop the previous watching thread before issuing another. You could create a new array and add the watchers that you created, then call cetcd_multi_watch_async and save the wid it returned. Stop all the wids when you really don't need them.

[xiaoyulei]

@shafreeck sorry, I lost one line cetcd_add_watcher(&m_watchers, watcher);, but in my real code it exist, I have update it.

[xiaoyulei]

If I do like this, it will generate a new thread. I want use one thread to manager all the watchers.

The fix is that you do not have to stop the previous watching thread before issuing another. You could create a new array and add the watchers that you created, then call cetcd_multi_watch_async and save the wid it returned. Stop all the wids when you really don't need them.

[xiaoyulei]

In cetcd_multi_watch_async_stop, I see the code just stop thread, not recycle watchers.

[shafreeck]

@YuleiXiao

In cetcd_multi_watch_async_stop, I see the code just stop thread, not recycle watchers.

Yes, you are right, sorry I made a mistake. However, cetcd_multi_watch_async_stop may cause some memory leaks in your case. It is recommended to use only when process going to exit.

I'll try to debug about your issue, thanks for feedback.

[shafreeck]

@YuleiXiao
I did some test and found that it is because the watcher's curl handler has been added to mcurl in curl_multi_watch

for (i = 0; i < count; ++i) {
         watcher = cetcd_array_get(watchers, i);
         curl_easy_setopt(watcher->curl, CURLOPT_PRIVATE, watcher);
         curl_multi_add_handle(mcurl, watcher->curl);
}

I think this may cause some inconsistent of watcher->curl's state when forcing to cancel the thread without removing it first.

I don't have any good ideas about this so far, maybe we should use pthread_cleanup_pop to do some cleanup.

[xiaoyulei]

@shafreeck I think you are right. we should cleanup curl before next used.