The DoS issue IS an important one, but this isn't the reason for that. The 10ms sleep was an arbitrary selection that allowed us to properly release the lock and permit the using-application to actually make calls.
There WAS an elegant solution pre-CA, though we still need to come up with one for the CA code. I agree we should be processing a little faster, however limiting to only 1 at a time is a good thing for back-pressure reasons. We likely should be doing the opposite in fact, and making the queue reject connections/messages once it hits a 'full' value. On Tue, 2015-04-07 at 20:44 +0000, Light, John J wrote: > That's not the only problem there. IIRC, the sleep is 10 milliseconds. > Since the message pump only handles one message per loop, that limits our > input rate to one message every 10 milliseconds. When I submitted this as a > comment to CA branch, I was informed that was to mitigate a denial of service > attack from consuming the message pump thread. (I suspect that answer was > made up on the spot. :-) If we face a DoS attack, the queue will now fill > with unprocessed entries and since only one message is being removed from it > every 10 milliseconds, we will consume all available memory building queue > entries. > > We should solve the non-DoS case (by draining the message queue every 10 > milliseconds), and then put together a comprehensive DoS defense. > > I suspect this problem is not being solved because the elegant solution is > not available yet. > > John Light > > > > -----Original Message----- > From: iotivity-dev-bounces at lists.iotivity.org [mailto:iotivity-dev-bounces > at lists.iotivity.org] On Behalf Of Keane, Erich > Sent: Tuesday, April 07, 2015 1:33 PM > To: Lenahan, Charlie > Cc: iotivity-dev at lists.iotivity.org > Subject: Re: [dev] IoTivity 0.9.0 - build sample application for sensors > > On Tue, 2015-04-07 at 20:25 +0000, Lenahan, Charlie wrote: > > On 4/7/15, 2:38 PM, "Thiago Macieira" <thiago.macieira at intel.com> wrote: > > > > >On Tuesday 07 April 2015 11:24:05 Kesavan, Vijay S wrote: > > >> There are sample applications to showcase the usage of the C-SDK at > > >> resource/csdk/stack/samples > > > > > >Ah, I see. There's a reason I didn't find them: they're written in > > >C++. I was looking for C sources... > > > > > >Anyway, I had the qualification of "good" examples. I don't consider > > >this a good example: > > > > > >static std::string putPayload = "{\"state\":\"on\",\"power\":5}"; > > > > > >The payload is hardcoded! An example application should be teaching > > >how to do things right... > > > > > >This sample application is also designed to be a testcase, as seen by > > >a variable (!) called TEST_CASE. That has implications on what the > > >application does and doesn't do, since it's not designed to be a > > >teaching tool. > > > > > >Another part I feel quite strongly about is this: > > > > > > while (!gQuitFlag) { > > > > > > if (OCProcess() != OC_STACK_OK) { > > > OC_LOG(ERROR, TAG, "OCStack process error"); > > > return 0; > > > } > > > > > > sleep(2); > > > } > > > > > >No example, under any circumstances, should EVER use the "sleep" > > >function. > > >There are very few valid uses for the > > >sleep/msleep/usleep/nanosleep/etc family of functions and most novice > > >developers will not know them[*]. We need to write our examples in > > >the way that real applications should be using the stack and clearly > > >this isn't it. > > > > > >[*] given that this is in our code, it's clear also that many > > >advanced developers don't know either. > > >-- > > > > In the C++ SDK (InProcClientWrapper , > > InProcServerWrapper)::listeningFunc > > has the same pattern. > > > > My understanding (before the CA branch) was that it was because the > > OCProcess() wasn?t using any mutex?s/conditional variable So the sleep > > was a hack to prevent a pegged thread. > > > > Looking in the bowls os OCProcess, it looks like it is now using a > > mutex, but the sleeps are still there. > > > > The C++ SDK listeningFunc does that by necessity. The OCProcess function is > NOT thread safe (OCProcessPresence contains global state which interferes > with other calls). So these are required to be protected from thread issues > via by the caller. Since OCProcess is a message-pump style call, it is > necessary to call this often, so listeningFunc does this in a loop. > > However, since OCProcess is NOT a blocking call, this will very quickly cause > performance issues if a sleep is not included. Otherwise this loop manages > to starve the rest of the stack. > > Sachin and I discussed making OCProcess be a blocking call at one point, and > had a good plan for how to do this (to make the sleep calls unnecessary), > however the implementation was tabled once the CA effort began. > > > > > > > _______________________________________________ > > iotivity-dev mailing list > > iotivity-dev at lists.iotivity.org > > https://lists.iotivity.org/mailman/listinfo/iotivity-dev > > _______________________________________________ > iotivity-dev mailing list > iotivity-dev at lists.iotivity.org > https://lists.iotivity.org/mailman/listinfo/iotivity-dev
