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

Dedicated server crashes #1289

Closed
crabmusket opened this Issue Apr 27, 2015 · 6 comments

Comments

Projects
None yet
2 participants
@crabmusket
Contributor

crabmusket commented Apr 27, 2015

3.7 release candidate is showing some crashes with the dedicated server as reported here. Symptoms in Linux are slightly different - the dedicated server console is unresponsive, but the engine still functions normally, and can act as a host.

@crabmusket crabmusket added the Bug label Apr 27, 2015

@crabmusket crabmusket added this to the 3.7 milestone Apr 27, 2015

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Apr 30, 2015

Contributor

Problem is in here:

void postConsoleInput( RawData data )
{
   // Schedule this to happen at the next time event.
   ConsoleValueRef argv[2];
   argv[0] = "eval";
   argv[1] = ( const char* ) data.data;
   Sim::postCurrentEvent(Sim::getRootGroup(), new SimConsoleEvent(2, argv, false));
}

The first assignment causes an assert in ConsoleValueRef::operator=. @jamesu? This pattern seems to be used in a couple of places. Wondering if this is a correct use of ConsoleValueRef. The comment on the class indicates:

// Can point to existing console variables,
// or act like a free floating value.

But I see no ways to construct a new value for it, only to assign it from an existing ConsoleValue.

Contributor

crabmusket commented Apr 30, 2015

Problem is in here:

void postConsoleInput( RawData data )
{
   // Schedule this to happen at the next time event.
   ConsoleValueRef argv[2];
   argv[0] = "eval";
   argv[1] = ( const char* ) data.data;
   Sim::postCurrentEvent(Sim::getRootGroup(), new SimConsoleEvent(2, argv, false));
}

The first assignment causes an assert in ConsoleValueRef::operator=. @jamesu? This pattern seems to be used in a couple of places. Wondering if this is a correct use of ConsoleValueRef. The comment on the class indicates:

// Can point to existing console variables,
// or act like a free floating value.

But I see no ways to construct a new value for it, only to assign it from an existing ConsoleValue.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Apr 30, 2015

Contributor

Further, for example, this constructor:

ConsoleValueRef::ConsoleValueRef(const char *newValue) : value(NULL)
{
   *this = newValue;
}

Will cause this to happen immediately:

ConsoleValueRef& ConsoleValueRef::operator=(const char *newValue)
{
   AssertFatal(value, "value should not be NULL");
   value->setStringValue(newValue);
   return *this;
}

...won't it?

Contributor

crabmusket commented Apr 30, 2015

Further, for example, this constructor:

ConsoleValueRef::ConsoleValueRef(const char *newValue) : value(NULL)
{
   *this = newValue;
}

Will cause this to happen immediately:

ConsoleValueRef& ConsoleValueRef::operator=(const char *newValue)
{
   AssertFatal(value, "value should not be NULL");
   value->setStringValue(newValue);
   return *this;
}

...won't it?

@jamesu

This comment has been minimized.

Show comment
Hide comment
@jamesu

jamesu Apr 30, 2015

Contributor

Indeed, the ConsoleValueRef should point to a ConsoleValue. e.g.

ConsoleValue values[2];
ConsoleValueRef refValues[2];

values[0].init();
values[0].setStringValue("eval");
values[1].init();
values[1].setStringValue((const char*)data.data);
refValues[0].value = &values[0];
refValues[1].value = &values[1];

This behaviour was previously changed due to the previous issues regarding storing new stack values in ConsoleValueRef.

A more simple method would probably be:

Con::executef("eval", (const char*)data.data);
Contributor

jamesu commented Apr 30, 2015

Indeed, the ConsoleValueRef should point to a ConsoleValue. e.g.

ConsoleValue values[2];
ConsoleValueRef refValues[2];

values[0].init();
values[0].setStringValue("eval");
values[1].init();
values[1].setStringValue((const char*)data.data);
refValues[0].value = &values[0];
refValues[1].value = &values[1];

This behaviour was previously changed due to the previous issues regarding storing new stack values in ConsoleValueRef.

A more simple method would probably be:

Con::executef("eval", (const char*)data.data);
@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Apr 30, 2015

Contributor

Okay, I get it. I figured something like that was the case, but the intent of the code is unclear. How do you feel about my second comment? Is the const char* constructor a bug?

Contributor

crabmusket commented Apr 30, 2015

Okay, I get it. I figured something like that was the case, but the intent of the code is unclear. How do you feel about my second comment? Is the const char* constructor a bug?

@jamesu

This comment has been minimized.

Show comment
Hide comment
@jamesu

jamesu May 6, 2015

Contributor

Encountering this issue with the oculus stuff in a windowed console. Is anyone making a patch or should I do it?

Contributor

jamesu commented May 6, 2015

Encountering this issue with the oculus stuff in a windowed console. Is anyone making a patch or should I do it?

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket May 7, 2015

Contributor

I was going to, but my time has been scant recently. Thanks for that!

Contributor

crabmusket commented May 7, 2015

I was going to, but my time has been scant recently. Thanks for that!

@crabmusket crabmusket closed this May 7, 2015

RichardRanft added a commit to RichardRanft/Torque3D that referenced this issue Jul 25, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment