Skip to content

Comments

Prevent Matlab on OS X from crashing on error#1406

Merged
shelhamer merged 1 commit intoBVLC:devfrom
CellScope:matcaffe-osx-fix-crash-on-error
Dec 8, 2014
Merged

Prevent Matlab on OS X from crashing on error#1406
shelhamer merged 1 commit intoBVLC:devfrom
CellScope:matcaffe-osx-fix-crash-on-error

Conversation

@dgolden1
Copy link
Contributor

@dgolden1 dgolden1 commented Nov 6, 2014

On my system, when using Matcaffe, if I encounter an error in the matcaffe mex file, Matlab crashes. This is ridiculous, and I figured out that the problem is in the LOG(FATAL) and CHECK() macros. When LOG(FATAL) is hit or a CHECK() fails, this crashes Matlab. My fix is to replace all instances of LOG(FATAL) and CHECK() with an inline function that calls LOG(ERROR) and mexErrMsgTxt. This returns control to Matlab with a proper error message instead of crashing.

My system:

  • OS X 10.9
  • XCode 6
  • Matlab 2014b (but matcaffe is compiled with Matlab 2014a)

Replace CHECK() and LOG(FATAL) with LOG(ERROR) and mexErrMsgTxt

A failed CHECK() or LOG(FATAL) causes Matlab to crash on OS X 10.9 with Matlab 2014a.
@dgolden1
Copy link
Contributor Author

dgolden1 commented Nov 6, 2014

A disadvantage of decomposing the error messaging into its own function is that you lose information about the source line in the LOG(ERROR) message. If desired, I can un-decompose the mex_error function for that reason, but it would involve a lot more copy-pasted code.

@sguada
Copy link
Contributor

sguada commented Nov 6, 2014

Look at matcaffe2 #501 for a possible solution.

Sergio

2014-11-05 17:56 GMT-08:00 Daniel Golden notifications@github.com:

A disadvantage of decomposing the error messaging into its own function is
that you lose information about the source line in the LOG(ERROR)
message. If desired, I can un-decompose the mex_error function for that
reason, but it would involve a lot more copy-pasted code.


Reply to this email directly or view it on GitHub
#1406 (comment).

@dgolden1
Copy link
Contributor Author

dgolden1 commented Nov 6, 2014

@sguada, thanks for the tip.

Help me understand your preprocessor macros a little more.

For example, in this one, does the error message have to be a string literal?

#define CHECK_EQ_3(a, b, m)  do {                               \
  if ( (a) != (b) ) {                                           \
    fprintf(stderr, "%s:%d: Check failed because %s != %s\n",   \
            __FILE__, __LINE__, #a, #b);                        \
    fprintf(stderr, "%s:%d: %s\n",                              \
            __FILE__, __LINE__, #m);                            \
    mexErrMsgTxt(#m);                                           \
  }                                                             \
} while (0);

Is there a way to adapt that to accept a stream like how glog's LOG works? Otherwise, it's really hard to, for example, make a long error message because I bump into the 80 character limit.

@sguada
Copy link
Contributor

sguada commented Nov 6, 2014

Yeah I think needs to be a string. But I guess you could build the string
to be as long as you need and the pass it as parameter.

Sergio

2014-11-05 21:21 GMT-08:00 Daniel Golden notifications@github.com:

@sguada https://github.com/sguada, thanks for the tip.

Help me understand your preprocessor macros a little more.

For example, in this one, does the error message have to be a string
literal?

#define CHECK_EQ_3(a, b, m) do {
if ( (a) != (b) ) {
fprintf(stderr, "%s:%d: Check failed because %s != %s\n",
FILE, LINE, #a, #b);
fprintf(stderr, "%s:%d: %s\n",
FILE, LINE, #m);
mexErrMsgTxt(#m);
}
} while (0);

Is there a way to adapt that to accept a stream like how glog's LOG
works? Otherwise, it's really hard to, for example, make a long error
message because I bump into the 80 character limit.


Reply to this email directly or view it on GitHub
#1406 (comment).

@dgolden1
Copy link
Contributor Author

dgolden1 commented Nov 6, 2014

But I think #m takes the name of the parameter as a string, not the content.

So if I say:

std::string msg_var = "The error message";
CHECK_EQ_3(0, 1, msg_var) ;

this would return an error of "msg_var", not "The error message", since msg_var isn't being evaluated.

I'll ponder this a little more, but my inclination is to suggest not doing much more work on this PR and merging as-is, since it's already better than the status quo and it will (hopefully) soon be replaced by #501 anyway.

@shelhamer
Copy link
Member

Merging to stop the unreasonable crashes as a similar choice was made for common errors in pycaffe.

Of course, matcaffe 2 #501 will improve many aspects of the MATLAB wrapper. @sguada any chance to look at the last changes from review and finish it up?

@drdan14 there's nothing specific to OS X here, no?

shelhamer added a commit that referenced this pull request Dec 8, 2014
Prevent Matlab on OS X from crashing on error
@shelhamer shelhamer merged commit 1f13b17 into BVLC:dev Dec 8, 2014
@dgolden1
Copy link
Contributor Author

dgolden1 commented Dec 8, 2014

I'm sure I've tested it on OS X, and I think I also tested it on Ubuntu 14.04, but I don't remember. The code should all be system independent.

@dgolden1 dgolden1 deleted the matcaffe-osx-fix-crash-on-error branch December 8, 2014 23:36
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.

3 participants